-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: show type of selection #966
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
georgefst
approved these changes
Jun 8, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, modulo styling.
This upstream version has support for displaying the selection's type or kind. This commit includes the required changes to node flavors, but the new API method isn't hooked up yet. Signed-off-by: Drew Hess <src@drewhess.com>
In subsequent commits, the info that was meant to be displayed in this panel will move to a tab in the PIP. (Note that no such info was actually ever displayed in this panel, because it was never implemented.) The `Sidebar` now only displays types & definitions. Signed-off-by: Drew Hess <src@drewhess.com>
We add tabs to the PIP, and add the new type info panel as a new tab in addition to the existing eval view. The styling here needs a lot of work, but this is mostly functional, except for 2 known issues: * Upon initial load, even if there's a selection, it doesn't show up in the info panel for some reason. * The types of holes are shown as a hole themselves. This is due to a backend bug that we haven't figured out yet. All other types I've tried seem to be correct. Signed-off-by: Drew Hess <src@drewhess.com>
Signed-off-by: Drew Hess <src@drewhess.com>
Now that the `EvalFull` component isn't always rendered (i.e., if the selection info tab is selected in the PIP), it can't persist the currently selected definition. However, the `Edit` component knows it, so we pass it down to `EvalFull` via a new property. (The whole eval roundtrip from `EvalFull` back up to `Edit` is pretty convoluted, and we should figure out a better way to do this.) Signed-off-by: Drew Hess <src@drewhess.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note that in this PR, we've lost the ability to drag the PIP around the canvas, not because it's broken, but just because there's now no good handle for starting the drag. The
@neodrag/react
functionality we're using passes themousedown
event to the component underneath, so if we make the new tab bar the drag handle, it could end up switching tabs, which is really annoying.I played around for an hour or two with some ideas for a handle, but didn't come up with anything great. At this point I'm just going to merge this so that we can lock in the ability to see selection types (modulo the issues with the backend not properly identifying the type for holes, see hackworthltd/primer#1062 (comment) for details).
I'll revisit the drag control later today, if I have a chance.