-
-
Notifications
You must be signed in to change notification settings - Fork 322
Add ability to rename nodes #2749
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
Conversation
I don't think the UX with renaming like that is too bad. There are just a few things that need a bit more polish:
|
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.
Sorry, I didn't mean to approve.
|
Okay, I can make the text input be focused and selected, but it's still not good enough. Chakra reaaally messes with focus in ways that I don't understand. Even if the text box is focused, simply hovering over another menu item causes it to lose focus. I honestly think we need a different input method. I can think of 2 alternatives:
Approach 1 is intuitive, but it (1) changes the meaning of double-clicking a node (not a problem IMO) and (2) has the accessibility problem that the size of the text in the text box depends on the zoom level. Approach 2 has none of these issues, but might be harder to implement, I'm not sure. What do you think? |
Approach 1 is what I initially tried but I had some trouble getting it to work, but I could try again I suppose |
Instead of adding review comments, I made you a PR with what I would have changed. Please test it to see whether you like the behavior. #2752 |
I'm not super thrilled with the way i ended up doing this (in regard to the text input being in the menu) but it was the easiest way.