-
Notifications
You must be signed in to change notification settings - Fork 5
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(RichTextEditor): flyout modals not closing when clicked outside #1732
Conversation
🦋 Changeset detectedLatest commit: 01399a1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for fondue-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for beta-fondue-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
setIsBlurObserverActive(true); | ||
}; | ||
|
||
richTextEditor.addEventListener('focusin', setupBlurObserver); |
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.
Shall we unsubscribe on focusout? Otherwise click in => click out => click in will have 2 listeners for the same thing, right?
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.
click out should always trigger the unsubscribe from the hook above
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.
adding the focusout handler means repeating a lot of the conditional checks that are already prevent in the document event listener (check if its in a modal or the toolbar etc). It seems like it would make sense for it to unsubscribe itself
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.
yeah my bad
Lead time: 3 hours, 1 minutes, 32 seconds (3.03 total hours) from first commit to close.
|
Fixes an issue where toolbar flyouts would remain open when the user clicked a different spot inside the RichTextEditor.
Also reduces the number of calls and the complexity of the blur observer by only activating it when the editor is in focus.
Linked pr: Frontify/brand-sdk#803
How to test with a guideline-block:
To be sure that you are using local code add a console.log() to the RTE component. (It will only show in edit mode since view mode uses the serialized html)
In @frontify/fondue
In @frontify/brand-sdk
In @frontify/guideline-blocks