Skip to content

Visibility toggle for sketch blocks in feature tree#10744

Merged
pierremtb merged 15 commits intomainfrom
pierremtb/issue10742-feature-tree-integration
Apr 7, 2026
Merged

Visibility toggle for sketch blocks in feature tree#10744
pierremtb merged 15 commits intomainfrom
pierremtb/issue10742-feature-tree-integration

Conversation

@pierremtb
Copy link
Copy Markdown
Contributor

@pierremtb pierremtb commented Apr 2, 2026

Closes #10742. Stacked on #10743

If people are happy with this we may consider applying the hide(sketch###) directly on extrusion.

Clipboard-20260406-232445-925.mp4

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
modeling-app Ready Ready Preview, Comment Apr 6, 2026 11:41pm

Request Review

@pierremtb pierremtb linked an issue Apr 2, 2026 that may be closed by this pull request
Base automatically changed from pierremtb/issue10742-ability-to-hide-sketch-block-ouput to main April 2, 2026 22:08
@jacebrowning jacebrowning marked this pull request as ready for review April 2, 2026 22:39
@jacebrowning jacebrowning requested a review from a team as a code owner April 2, 2026 22:39
@pierremtb pierremtb marked this pull request as draft April 3, 2026 14:06
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Codex wrote those, think they're alright?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice yeah even added coverage to the helix hide behavior, thanks.

pathArtifact: Extract<Artifact, { type: 'path' }>,
artifactGraph: ArtifactGraph
): Extract<Artifact, { type: 'sketchBlock' }> | undefined {
// TODO: replace this pathToNode match once the artifact graph can map a Path to its SketchBlock directly.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is wrapped nicely but still isn't a great way to find the sibbling. Keeping the TODO for now.

@pierremtb pierremtb marked this pull request as ready for review April 6, 2026 23:29
@pierremtb pierremtb requested a review from franknoirot April 7, 2026 09:39
@pierremtb pierremtb enabled auto-merge (squash) April 7, 2026 12:59
Copy link
Copy Markdown
Contributor

@franknoirot franknoirot left a comment

Choose a reason for hiding this comment

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

Works like a charm, thanks for this it was a miss on our planning!

Image

item,
operations: kclManager.operations ?? [],
artifactGraph: kclManager.artifactGraph,
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is cool

return null
}

export function hasSamePathToNode(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

c: I'm a little surprised we don't have a helper for this already

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same!

pathArtifact.codeRef.pathToNode,
artifactGraph
).find(
(artifact): artifact is Extract<Artifact, { type: 'sketchBlock' }> =>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

c: I like this type predicate with the is Extract, we should use this more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed! Was from Codex and I found it great.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice yeah even added coverage to the helix hide behavior, thanks.

targetArtifact?: Artifact
}

export function resolveFeatureTreeVisibility(input: {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will be helpful to use in the bodies pane, nice encapsulation.

@pierremtb pierremtb merged commit fdd3b50 into main Apr 7, 2026
77 of 78 checks passed
@pierremtb pierremtb deleted the pierremtb/issue10742-feature-tree-integration branch April 7, 2026 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ability to hide sketch block ouput

3 participants