fix(drawing): Fix Safari doesn't focus and blur color picker button #1321
Conversation
mxiao6
commented
Jan 20, 2021
mxiao6
commented
- Unit test
- Cross-browser testing
mxiao6
force-pushed
the
fix-safari-picker
branch
from
b1b796b to
60fd033
Compare
| if (event.currentTarget.focus) { | ||
| // Buttons do not receive focus in Firefox and Safari on MacOS | ||
| event.currentTarget.focus(); | ||
| // When focus() is called, preventDefault must be called to keep the focus from leaving the target |
There was a problem hiding this comment.
When focus is called within a mousedown handler, just for clarity?
There was a problem hiding this comment.
Could this be done in handleClick as well?
There was a problem hiding this comment.
It doesn't affect the current flow, but affects the second click of the button. When you click the button the second time, handle blur happens before handle click, which sets the state to false and back to true again. so you cannot close the palette anymore.
There was a problem hiding this comment.
The interesting thing here is that even if you click the same button which has the focus, it will still blur and re-focus again, except you handle the logic before everything in handleMouseDown
| }, | ||
| preventDefault: jest.fn(), | ||
| }; | ||
| (getToggleButton(getWrapper()).prop('onMouseDown') as Function)(mockEvent); |
There was a problem hiding this comment.
I've been favoring simulate because it avoids the need to cast. I've seen the Jest folks say not to use it, but it's there and it works well. Up to you.
There was a problem hiding this comment.
Yeah I prefer using simulate too, and I tried it first but it doesn't work (I googled it why but it seems sometimes it happens and everyone just use props.onClick() etc. ). Then I have to use this fallback plan.
