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.

Sign up for GitHub

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

Jump to bottom

Allow LexicalTypeaheadMenuPlugin to work when inside an iframe #5044

Merged
merged 3 commits into from
Sep 28, 2023
Merged

Allow LexicalTypeaheadMenuPlugin to work when inside an iframe #5044

merged 3 commits into from
Sep 28, 2023

Conversation

lestertay
Copy link
Contributor

@lestertay lestertay commented Sep 25, 2023 •
edited
Loading

Hi Lexical team, I'm making some changes to the LexicalTypeaheadMenuPlugin such that when we're using the ContentEditable in an iframe but rendering the dropdown menu of the typeahead outside (iframe's parent), we'll still calculate the position of the dropdown menu correctly.

@vercel
Copy link

vercel bot commented Sep 25, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 27, 2023 2:18pm
lexical-playground ❌ Failed (Inspect) Sep 27, 2023 2:18pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 25, 2023
@lestertay lestertay marked this pull request as ready for review September 26, 2023 14:23
fantactuka
fantactuka approved these changes Sep 26, 2023
View reviewed changes
Copy link
Contributor

@fantactuka fantactuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this PR, but maybe we should add editor.getWindow() / editor.getDocument() with fallback to globals. It might simplify swapping globals with jsdom (e.g., for server rendering)

packages/lexical-react/src/LexicalTypeaheadMenuPlugin.tsx Outdated Show resolved Hide resolved
@ivailop7
Copy link
Collaborator

Unrelated to this PR, but maybe we should add editor.getWindow() / editor.getDocument() with fallback to globals. It might simplify swapping globals with jsdom (e.g., for server rendering)

I think, this is a great idea.

zurfyx
zurfyx reviewed Sep 26, 2023
View reviewed changes
packages/lexical-react/src/LexicalTypeaheadMenuPlugin.tsx Outdated
@@ -236,7 +236,8 @@ export function LexicalTypeaheadMenuPlugin<TOption extends MenuOption>({
useEffect(() => {
const updateListener = () => {
editor.getEditorState().read(() => {
const range = document.createRange();
const _window = editor._window ?? window;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lestertay @fantactuka - I wonder if editor._window should always be set to avoid the fallback to the ?? window in getDefaultView that powers this setter https://github.com/facebookexternal/Outline//blob/02f01f160e8894f133e076ab5e38e7a3fd6a5f7a/packages/lexical/src/LexicalEditor.ts#L961-L967

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fantactuka i think that'll be useful and a sensible fallback. I'll be happy to make another PR to enable this since i've seen quite a few places within Lexical using editor._window.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that was an idea for a follow up 👍

@lestertay @fantactuka
Update packages/lexical-react/src/LexicalTypeaheadMenuPlugin.tsx
30dfea8 
Co-authored-by: Maksim Horbachevsky <fantactuka@gmail.com>
@lestertay lestertay merged commit f0933ba into facebook:main Sep 28, 2023
36 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zurfyx zurfyx zurfyx left review comments

@fantactuka fantactuka fantactuka approved these changes

@acywatson acywatson Awaiting requested review from acywatson acywatson is a code owner

@thegreatercurve thegreatercurve Awaiting requested review from thegreatercurve

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lestertay @ivailop7 @fantactuka @zurfyx @facebook-github-bot