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

Format Library: Explore a fundamental solution regarding highlight popover jumping #54909

Closed
t-hamano opened this issue Sep 28, 2023 · 4 comments
Labels
[Feature] Colors Color management [Package] Format library /packages/format-library [Type] Enhancement A suggestion for improvement.

Comments

@t-hamano
Copy link
Contributor

t-hamano commented Sep 28, 2023

Issue #49286 where popovers jump when applying inline highlighting to text has been fixed in #54736.

However, I am filing this issue in order to find a more fundamental solution.

Maybe we could start by disabling caching completely and seeing what happens?
Comment from @ciampo

@ciampo
Copy link
Contributor

ciampo commented Sep 28, 2023

A few ideas:

  • let's disable the caching altogether, it may help to get to the root of the problem
  • I have a feeling that the main issue is that, when switching from "no highlight" to "highlight", the popover anchor may be removed from the DOM (or somewhat altered). In that case, a few possible solutions (at a high-level):
    • [current solution] caching the previous anchor position — that means that, even if the DOM element changes/is removed, we're going to continue using the previous anchor's position. It is a bit hacky, but it works
    • update the popover anchor to point to the most up-to-date element (that would require us to look into how highlighting is implemented, and somehow grab a reference to the correct DOM element)
    • tweak the highlight functionality so that DOM elements are not added/removed (this may be harder)

Let me know how the exploration goes!

@jordesign jordesign added the [Type] Enhancement A suggestion for improvement. label Sep 28, 2023
@t-hamano
Copy link
Contributor Author

t-hamano commented Jul 2, 2024

This issue may have already been addressed by #58900. It looks like the cache has been cleared as well.

Can we close this issue?

@ciampo
Copy link
Contributor

ciampo commented Jul 2, 2024

That's great to hear! Let's double-check by asking directly @jeryj , who implemented the fix you mentioned :)

@jeryj
Copy link
Contributor

jeryj commented Jul 2, 2024

Yup! The fix in #58900 should have addressed this.

@jeryj jeryj closed this as completed Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Colors Color management [Package] Format library /packages/format-library [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

4 participants