Skip to content

Right click a feature on the model to center the code pane on it #7447 #7664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 146 additions & 1 deletion src/components/ViewControlMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,38 @@
const shouldLockView =
modelingState.matches('Sketch') &&
!settings.app.allowOrbitInSketchMode.current

// Check if there's a valid selection with source range for "View KCL source code"
const hasValidSelection = useMemo(() => {

Check failure on line 35 in src/components/ViewControlMenu.tsx

View workflow job for this annotation

GitHub Actions / npm-tsc

Cannot redeclare block-scoped variable 'hasValidSelection'.
const selections = modelingState.context.selectionRanges.graphSelections
return (
selections.length > 0 &&
selections.some((selection) => {
return (
selection.codeRef?.range &&
selection.codeRef.range[0] !== undefined &&
selection.codeRef.range[1] !== undefined
)
})
)
}, [modelingState.context.selectionRanges.graphSelections])


// Check if there's a valid selection with source range for "View KCL source code"
const hasValidSelection = useMemo(() => {

Check failure on line 51 in src/components/ViewControlMenu.tsx

View workflow job for this annotation

GitHub Actions / npm-tsc

Cannot redeclare block-scoped variable 'hasValidSelection'.
const selections = modelingState.context.selectionRanges.graphSelections
return (
selections.length > 0 &&
selections.some((selection) => {
return (
selection.codeRef?.range &&
selection.codeRef.range[0] !== undefined &&
selection.codeRef.range[1] !== undefined
)
})
)
}, [modelingState.context.selectionRanges.graphSelections])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There appears to be a duplicate implementation of the hasValidSelection function in the useViewControlMenuItems hook. The same code block is defined twice (lines 35-47 and 51-63), with identical logic checking for valid selections with source ranges. This duplication will cause the second definition to override the first one.

Consider removing one of these duplicate implementations to avoid potential confusion and maintain clean code.

Suggested change
// Check if there's a valid selection with source range for "View KCL source code"
const hasValidSelection = useMemo(() => {
const selections = modelingState.context.selectionRanges.graphSelections
return (
selections.length > 0 &&
selections.some((selection) => {
return (
selection.codeRef?.range &&
selection.codeRef.range[0] !== undefined &&
selection.codeRef.range[1] !== undefined
)
})
)
}, [modelingState.context.selectionRanges.graphSelections])
// Check if there's a valid selection with source range for "View KCL source code"
const hasValidSelection = useMemo(() => {
const selections = modelingState.context.selectionRanges.graphSelections
return (
selections.length > 0 &&
selections.some((selection) => {
return (
selection.codeRef?.range &&
selection.codeRef.range[0] !== undefined &&
selection.codeRef.range[1] !== undefined
)
})
)
}, [modelingState.context.selectionRanges.graphSelections])
// Check if there's a valid selection with source range for "View KCL source code"
const hasValidSelection = useMemo(() => {
const selections = modelingState.context.selectionRanges.graphSelections
return (
selections.length > 0 &&
selections.some((selection) => {
return (
selection.codeRef?.range &&
selection.codeRef.range[0] !== undefined &&
selection.codeRef.range[1] !== undefined
)
})
)
}, [modelingState.context.selectionRanges.graphSelections])

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.


const menuItems = useMemo(
() => [
...Object.entries(VIEW_NAMES_SEMANTIC).map(([axisName, axisSemantic]) => (
Expand Down Expand Up @@ -64,36 +96,149 @@
>
Center view on selection
</ContextMenuItem>,
<ContextMenuItem
onClick={() => {
// Get the first valid selection with a source range
const selection =
modelingState.context.selectionRanges.graphSelections.find(
(sel) =>
sel.codeRef?.range &&
sel.codeRef.range[0] !== undefined &&
sel.codeRef.range[1] !== undefined
)

if (selection?.codeRef?.range) {
// First, open the code pane if it's not already open
if (!modelingState.context.store.openPanes.includes('code')) {
modelingSend({
type: 'Set context',
data: {
openPanes: [...modelingState.context.store.openPanes, 'code'],
},
})
}

// Navigate to the source code location
modelingSend({
type: 'Set selection',
data: {
selectionType: 'singleCodeCursor',
selection: {
artifact: selection.artifact,
codeRef: selection.codeRef,
},
scrollIntoView: true,
},
})
}
}}
disabled={!hasValidSelection}
>
View KCL source code
</ContextMenuItem>,

Check warning on line 138 in src/components/ViewControlMenu.tsx

View workflow job for this annotation

GitHub Actions / semgrep-oss/scan

jsx-not-internationalized

JSX element not internationalized View KCL source code . You should support different languages in your website or app with internationalization. Instead use packages such as i18next in order to internationalize your elements.
<ContextMenuDivider />,
<ContextMenuItem
onClick={() => {
if (selectedPlaneId) {
sceneInfra.modelingSend({
type: 'Enter sketch',
data: { forceNewSketch: true },
})

const defaultSketchPlaneSelected =
selectDefaultSketchPlane(selectedPlaneId)
if (
!err(defaultSketchPlaneSelected) &&
defaultSketchPlaneSelected
) {
return
}

const artifact = kclManager.artifactGraph.get(selectedPlaneId)
void selectOffsetSketchPlane(artifact)
}
}}
disabled={!selectedPlaneId}
>
Start sketch on selection
</ContextMenuItem>,
<ContextMenuItem
onClick={() => {
// Get the first valid selection with a source range
const selection =
modelingState.context.selectionRanges.graphSelections.find(
(sel) =>
sel.codeRef?.range &&
sel.codeRef.range[0] !== undefined &&
sel.codeRef.range[1] !== undefined
)

if (selection?.codeRef?.range) {
// First, open the code pane if it's not already open
if (!modelingState.context.store.openPanes.includes('code')) {
modelingSend({
type: 'Set context',
data: {
openPanes: [...modelingState.context.store.openPanes, 'code'],
},
})
}

// Navigate to the source code location
modelingSend({
type: 'Set selection',
data: {
selectionType: 'singleCodeCursor',
selection: {
artifact: selection.artifact,
codeRef: selection.codeRef,
},
scrollIntoView: true,
},
})
}
}}
disabled={!hasValidSelection}
>
View KCL source code
</ContextMenuItem>,

Check warning on line 204 in src/components/ViewControlMenu.tsx

View workflow job for this annotation

GitHub Actions / semgrep-oss/scan

jsx-not-internationalized

JSX element not internationalized View KCL source code . You should support different languages in your website or app with internationalization. Instead use packages such as i18next in order to internationalize your elements.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There appears to be a duplicate implementation of the "View KCL source code" context menu item in the useViewControlMenuItems hook. The same functionality is implemented twice (once at lines 99-138 and again at lines 165-204), which would result in two identical menu items appearing in the context menu.

Please remove one of these duplicate implementations to avoid confusing users with redundant options.

Suggested change
<ContextMenuItem
onClick={() => {
// Get the first valid selection with a source range
const selection =
modelingState.context.selectionRanges.graphSelections.find(
(sel) =>
sel.codeRef?.range &&
sel.codeRef.range[0] !== undefined &&
sel.codeRef.range[1] !== undefined
)
if (selection?.codeRef?.range) {
// First, open the code pane if it's not already open
if (!modelingState.context.store.openPanes.includes('code')) {
modelingSend({
type: 'Set context',
data: {
openPanes: [...modelingState.context.store.openPanes, 'code'],
},
})
}
// Navigate to the source code location
modelingSend({
type: 'Set selection',
data: {
selectionType: 'singleCodeCursor',
selection: {
artifact: selection.artifact,
codeRef: selection.codeRef,
},
scrollIntoView: true,
},
})
}
}}
disabled={!hasValidSelection}
>
View KCL source code
</ContextMenuItem>,
<ContextMenuDivider />,
<ContextMenuItem
onClick={() => {
if (selectedPlaneId) {
sceneInfra.modelingSend({
type: 'Enter sketch',
data: { forceNewSketch: true },
})
const defaultSketchPlaneSelected =
selectDefaultSketchPlane(selectedPlaneId)
if (
!err(defaultSketchPlaneSelected) &&
defaultSketchPlaneSelected
) {
return
}
const artifact = kclManager.artifactGraph.get(selectedPlaneId)
void selectOffsetSketchPlane(artifact)
}
}}
disabled={!selectedPlaneId}
>
Start sketch on selection
</ContextMenuItem>,
<ContextMenuItem
onClick={() => {
// Get the first valid selection with a source range
const selection =
modelingState.context.selectionRanges.graphSelections.find(
(sel) =>
sel.codeRef?.range &&
sel.codeRef.range[0] !== undefined &&
sel.codeRef.range[1] !== undefined
)
if (selection?.codeRef?.range) {
// First, open the code pane if it's not already open
if (!modelingState.context.store.openPanes.includes('code')) {
modelingSend({
type: 'Set context',
data: {
openPanes: [...modelingState.context.store.openPanes, 'code'],
},
})
}
// Navigate to the source code location
modelingSend({
type: 'Set selection',
data: {
selectionType: 'singleCodeCursor',
selection: {
artifact: selection.artifact,
codeRef: selection.codeRef,
},
scrollIntoView: true,
},
})
}
}}
disabled={!hasValidSelection}
>
View KCL source code
</ContextMenuItem>,
<ContextMenuItem
onClick={() => {
// Get the first valid selection with a source range
const selection =
modelingState.context.selectionRanges.graphSelections.find(
(sel) =>
sel.codeRef?.range &&
sel.codeRef.range[0] !== undefined &&
sel.codeRef.range[1] !== undefined
)
if (selection?.codeRef?.range) {
// First, open the code pane if it's not already open
if (!modelingState.context.store.openPanes.includes('code')) {
modelingSend({
type: 'Set context',
data: {
openPanes: [...modelingState.context.store.openPanes, 'code'],
},
})
}
// Navigate to the source code location
modelingSend({
type: 'Set selection',
data: {
selectionType: 'singleCodeCursor',
selection: {
artifact: selection.artifact,
codeRef: selection.codeRef,
},
scrollIntoView: true,
},
})
}
}}
disabled={!hasValidSelection}
>
View KCL source code
</ContextMenuItem>,
<ContextMenuDivider />,
<ContextMenuItem
onClick={() => {
if (selectedPlaneId) {
sceneInfra.modelingSend({
type: 'Enter sketch',
data: { forceNewSketch: true },
})
const defaultSketchPlaneSelected =
selectDefaultSketchPlane(selectedPlaneId)
if (
!err(defaultSketchPlaneSelected) &&
defaultSketchPlaneSelected
) {
return
}
const artifact = kclManager.artifactGraph.get(selectedPlaneId)
void selectOffsetSketchPlane(artifact)
}
}}
disabled={!selectedPlaneId}
>
Start sketch on selection
</ContextMenuItem>,

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

<ContextMenuDivider />,
<ContextMenuItem
onClick={() => {
if (selectedPlaneId) {
sceneInfra.modelingSend({
type: 'Enter sketch',
data: { forceNewSketch: true },
})

const defaultSketchPlaneSelected =
selectDefaultSketchPlane(selectedPlaneId)
if (
!err(defaultSketchPlaneSelected) &&
defaultSketchPlaneSelected
) {
return
}

const artifact = kclManager.artifactGraph.get(selectedPlaneId)
void selectOffsetSketchPlane(artifact)
}
}}
disabled={!selectedPlaneId}
>
Start sketch on selection
</ContextMenuItem>,

Check warning on line 230 in src/components/ViewControlMenu.tsx

View workflow job for this annotation

GitHub Actions / semgrep-oss/scan

jsx-not-internationalized

JSX element not internationalized Start sketch on selection . You should support different languages in your website or app with internationalization. Instead use packages such as i18next in order to internationalize your elements.
<ContextMenuDivider />,
<ContextMenuItemRefresh />,
],
[VIEW_NAMES_SEMANTIC, shouldLockView, selectedPlaneId]

[
VIEW_NAMES_SEMANTIC,
shouldLockView,
selectedPlaneId, hasValidSelection,
modelingState.context.selectionRanges.graphSelections,
modelingState.context.store.openPanes,
]
)
return menuItems
}
Expand Down
Loading