-
-
Notifications
You must be signed in to change notification settings - Fork 295
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
Improved node resetting #2566
Improved node resetting #2566
Conversation
joeyballentine
commented
Feb 9, 2024
Not a fan. I always thought that the Reset option was somewhat useless, and you went and made 3 versions of it. So I'm not too keen on dedicating 25% of the space in the likely most important context menu of chainner to something like that. So could you please explain your motivation for adding these options? That said, I'm not strictly against adding these options (especially if there is a good use case), I'm only very against the current presentation. Assuming we keep the current options as is, I think putting them into a sub menu would be the best option. |
So i looked into creating a submenu. It's not something chakra supports natively, and as far as i can tell would be somewhat complicated to do, so i would prefer to just not do that now if that's ok with you. |
Maybe i could re-use the context menu stuff and manually open the context menu on hover? i was thinking we'd always want the submenu aligned with the parent menu, but if we're ok just sticking to the mouse that could work |
The code is super gross now, but it's in a submenu and it works as you would expect |
334feeb
to
4263645
Compare
Clicking on "Reset node" itself just closes the context menu. Clicking on it should do nothing. |
@RunDevelopment I tried to prevent that, but doing e.preventdefault and e.stoppropagation didn't do anything |
Co-authored-by: Michael Schmidt <[email protected]>