fix: modal escape key, timer race, scroll lock #364
Open
avinxshKD wants to merge 1 commit intoControlCore-Project:devfrom
Open
fix: modal escape key, timer race, scroll lock #364avinxshKD wants to merge 1 commit intoControlCore-Project:devfrom
avinxshKD wants to merge 1 commit intoControlCore-Project:devfrom
Conversation
Author
|
@pradeeban as ParentModal is the root of all 9 modals so fixing the 3 bugs here was enough. |
Author
|
Also @pradeeban, wanted to get your take. I'm a little confused between 'CONTROL-CORE usability enhancements' (Project 15) and 'BHV' project(Project 2). As I'm already comfortable in the concore codebase so usability enhancements which is project 15 feels natural for a second project to work on, but I'm also genuinely interested in BHV and familiar with its stack. Since I can only commit to one of them properly alongsight the main concore project, which would you say has more room for learning and more meaningful contribution for gsoc period? |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
onRequestClose was commented out so Escape never worked. Switched the setTimeout ID from useState to useRef so clearTimeout actually gets the right ID on fast open/close. Added body.overflow hidden/restore so the page doesn't scroll behind the overlay.
Also Cleanup return added to the effect so the timer doesn't fire on unmount.
Fixes #362