-
Notifications
You must be signed in to change notification settings - Fork 82
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: consolidate "help" elements into help menu #1118
base: main
Are you sure you want to change the base?
Conversation
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.
Good work so far, but there are still a couple of bugs I'm noticing:
- Help box does not re-open every time icon is clicked. Steps to repro:
- Click on help box icon
- X it out
- Click on help box icon again. It will not show, since clicking it just attempts to close the already-closed help box.
-
Mouse-off delay is slightly too long. I like the feature (great suggestion by @KevinWu098 ) but a collapse timeout of 1 second is quite long. Try out some smaller times and see how it feels. Maybe half a second?
-
Mouse-off delay only works from the blue circle. If there is no way to implement this natively with mui then we can settle with a short mouse-off delay and not address this, but it would be apt for the delay to also work from the icon buttons (right now it doesn't).
-
The tutorial dies if you mouse over the help menu while the tutorial is running.
- Click restart tutorial
- Mouse-off the menu
- Mouse back onto the menu once it closes
If you need help with implementation or have questions about code, let us know in the Discord ASAP
LGTM: light mode, help box design
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.
Just throwing up some comments to work on
@adcockdalton I've fixed everything and cleaned up my code it should be ready for a final review now. Let me know if anything comes up! |
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.
Using event emitters and listeners in RightPaneStore
is a broken window, but I'll let it slide because we'll be refactoring this.
@KevinWu098 @adcockdalton Can you check if your comments have been addressed? This has been sitting for a while |
mm, I had a comment about refactoring logic into a Zustand store specifically for the help elements, but that can be in a follow up -- don't think I have anything blocking now |
@vicksey
Nice PR! |
Summary
Aggregated the "help" elements, Help Box, Feedback Form, and Tutorial, into one help menu button. Once hovered the three elements are shown and can be clicked to be used.
Test Plan
Issues
Closes #1053