Skip to content
This repository was archived by the owner on Aug 1, 2025. It is now read-only.

fix(onebox): Do not focus editor when inserting/updating search results context#6385

Merged
vovakulikov merged 1 commit intomainfrom
fkling-srch-1483-when-clicking-allnone-checkbox-causes-jitter
Dec 18, 2024
Merged

fix(onebox): Do not focus editor when inserting/updating search results context#6385
vovakulikov merged 1 commit intomainfrom
fkling-srch-1483-when-clicking-allnone-checkbox-causes-jitter

Conversation

@fkling
Copy link
Copy Markdown
Contributor

@fkling fkling commented Dec 17, 2024

Closes srch-1483

Changing the focus behavior was more difficult than expected. Apparently when Lexical has a Selection then it will 'steal' the focus whenever changes to its content is made.

There have been made changes to Lexical just recently that would allow us to avoid this (facebook/lexical#6894) but we haven't updated to this version yet and updating this closely before the EAP seems risky.

I found a workaround in one of the discussions that suggests to set the selection to null when making a change.

In order to do this I refactored some of the function to make them more composable. Instead of calling editor.update directly these functions are now supposed to be called from editor.update.

Since I've added a helper for promisifying editor.update anyway, I also addressed one of the issues I've encountered in the last PR, namely that onUpdate is not called when the editor state doesn't change. To prevent accidentally calling it in a way that would not resolve the promise, the callback has to return a boolean.
This will btw also be fixed on the Lexical side by the above mentioned PR.

Test plan

Manual testing

sg-ob-follow-up-focus.mp4

…ts context

Changing the focus behavior was more difficult than expected. Apparently
when Lexical has a Selection then it will 'steal' the focus whenever
changes to its content is made.

There have been made changes to Lexical just recently that would allow
us to avoid this (facebook/lexical#6894) but we
haven't updated to this version yet and updating this closely before the
EAP seems risky.

I found a workaround in one of the discussions that suggests to set the
selection to `null` when making a change.

In order to do this I refactored some of the function to make them more
composable. Instead of calling `editor.update` directly these functions
are now supposed to be called from `editor.update`.

Since I've added a helper for creating a promise-version of
`editor.update` anyway, I also addressed one of the issues I've
encountered in the last PR, namely that `onUpdate` is not called when
the editor state doesn't change.
This will btw also be fixed on the Lexical side by the above mentioned PR.
@fkling fkling requested review from 0xnmn and vovakulikov December 17, 2024 11:53
Copy link
Copy Markdown
Contributor

@vovakulikov vovakulikov left a comment

Choose a reason for hiding this comment

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

Tested manually.

@vovakulikov vovakulikov merged commit fd52178 into main Dec 18, 2024
@vovakulikov vovakulikov deleted the fkling-srch-1483-when-clicking-allnone-checkbox-causes-jitter branch December 18, 2024 13:00
vovakulikov pushed a commit that referenced this pull request Dec 18, 2024
…ts context (#6385)

Closes srch-1483

Changing the focus behavior was more difficult than expected. Apparently
when Lexical has a Selection then it will 'steal' the focus whenever
changes to its content is made.

There have been made changes to Lexical just recently that would allow
us to avoid this (facebook/lexical#6894) but we
haven't updated to this version yet and updating this closely before the
EAP seems risky.

I found a workaround in one of the discussions that suggests to set the
selection to `null` when making a change.

In order to do this I refactored some of the function to make them more
composable. Instead of calling `editor.update` directly these functions
are now supposed to be called from `editor.update`.

Since I've added a helper for promisifying `editor.update` anyway, I
also addressed one of the issues I've encountered in the last PR, namely
that `onUpdate` is not called when the editor state doesn't change. To
prevent accidentally calling it in a way that would not resolve the
promise, the callback has to return a boolean.
This will btw also be fixed on the Lexical side by the above mentioned
PR.


## Test plan

Manual testing 


https://github.com/user-attachments/assets/296cca9e-b8e3-433e-a39a-befbb8fe2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants