Skip to content
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(rich-text-input): fix onBlur and onFocus #1093

Merged
merged 1 commit into from
Oct 11, 2019
Merged

Conversation

montezume
Copy link
Contributor

@montezume montezume commented Oct 8, 2019

Summary

Regression caused by #1088

I imagine that because we are using setTimeout, our synthetic event is gone and by not calling persist(), we no longer have access to name and id inside of the onBlur.

When I debug it in storybook I also get null as event.target for a non persisted event.

@montezume montezume requested review from tdeekens and jonnybel October 8, 2019 13:38
@montezume montezume added the 🐛 Type: Bug Something isn't working label Oct 8, 2019
@montezume montezume self-assigned this Oct 8, 2019
@montezume
Copy link
Contributor Author

I'm actually wondering now if we even need the setTimeout before calling onBlur. Playing around with it now.

@jonnybel
Copy link
Contributor

jonnybel commented Oct 8, 2019

Hmm, to recap, why was it there in the first place?

@montezume
Copy link
Contributor Author

@jonnybel because we were having problems with focus hijacking, but it may have been fixed by refactoring the component 🤔

@montezume
Copy link
Contributor Author

wtf is this

@montezume
Copy link
Contributor Author

We used to call setState and set an isFocused variable, maybe that was the issue? Now we don't do that anymore.

@montezume
Copy link
Contributor Author

If I remove the setTimeout, then you need to click twice to focus it under some circumstances.

@montezume montezume merged commit 8d76ba2 into master Oct 11, 2019
@montezume montezume deleted the ml-fix-on-blur branch October 11, 2019 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants