-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
Update EnhancedButton.js to prevent duplicate event processing #10108
Conversation
* Added Governance doc * Fixed hierarchy * Update GOVERNANCE.md Just for consistency with the other .md. Later, we can use prettier to format everything.
…InkBar (mui#9598) * Add support for inline style override for parent container of InkBar component inside Tabs * Update Tabs.js
while unmounting `this.track` is null and exception is raised. returning 0 offset when no ref.
Reinstated the keyUp handler. Added recording of the timestamp of events processed by handleClick to prevent processing of duplicated events. mui#9439 mui#9344 Background : The handleKeyUp firing of handleClick is duplicating the event on the processing queue. I've not been able to find the reason for the duplication, I suspect something lower down in the React stack... but I can't prove it either way. This will however stop a duplicated click event being processed more than once.
src/internal/EnhancedButton.js
Outdated
this.removeKeyboardFocus(event); | ||
this.props.onClick(event); | ||
} | ||
} |
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.
This function replaces handleFocus
? Also, there are 2 handleClick
functions. Maybe an oversight?
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.
The code puts back functionality dropped in the last update. The extra code stops the same event being handled multiple times.
There should be no replacement of any function.
I'm replying on my phone right now, but if you would like a full breakdown let me know and I will go through it when I get home tonight.
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.
@djbuckley I'm just looking at the diff... could be a typo?
This PR replaces handleFocus
- https://github.com/mui-org/material-ui/pull/10108/files#diff-0ab20af15832a25c3ce0468e2409753bR216
Also there is an extra handleClick
here - https://github.com/mui-org/material-ui/pull/10108/files#diff-0ab20af15832a25c3ce0468e2409753bR235
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.
yep, you are right I made a bit of a cross eyed boo boo there...
I would have pulled the version in my fork but that would have introduced more issues to check.
Sorry about being so ham fisted here.
pulled the click and focus event handlers from my working code to correct my accidental botch of the last insert
@hai-cea Are you good with this PR now? |
Reinstated the keyUp handler.
Added recording of the timestamp of events processed by handleClick to prevent processing of duplicated events. #9439 #9344
Background :
The handleKeyUp firing of handleClick is duplicating the event on the processing queue. I've not been able to find the reason for the duplication, I suspect something lower down in the React stack... but I can't prove it either way. This will however stop a duplicated click event being processed more than once.