-
Notifications
You must be signed in to change notification settings - Fork 14
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
fix(FEC-11520): Multi dropdowns are openable in cvaa overlay #638
Conversation
* @returns {void} | ||
* @memberof DropDown | ||
*/ | ||
onClick = (e: Event): void => { | ||
e.stopPropagation(); |
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.
what are the side effects of removing the stopPropagation? we have additional click handlers that we wouldn't want them to catch the event.
As only drop down is using the menu maybe we can lift the event listener up to dropdown and expose the handler as a callback prop?
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.
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.
if you mean to the click outside event i did lift it up and now it is managed from the dropdwon, But that in itself is not enough, the challenge is to distinguish between an event that comes from the current component and the rest and i did it by compering the event timestamps
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.
@OrenMe @JonathanTGold we need to remove the stopPropagation
for closing the other menus. I think it's safe as it has been added after the UI already had most of the click handlers. of course, we have to verify it after this change.
And I prefer the simple timeout solution rather than this event mechanism.
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.
Your call @yairans
Description of the Changes
issue: the stop propagation (on dropdown click event) prevent the close menus logic handler (which trigerd by docoment click listener) to be trigerd.
fix: remove the stop propagation and and move the dropdown opening action to the next event loop so that is would bot be canceled by on close logic
sovles: FEC-11520
CheckLists