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(inline-toolbar): appearance logic improved #2550

Merged
merged 8 commits into from
Dec 8, 2023
Merged

fix(inline-toolbar): appearance logic improved #2550

merged 8 commits into from
Dec 8, 2023

Conversation

neSpecc
Copy link
Member

@neSpecc neSpecc commented Dec 6, 2023

Problems

Right now we have several problems with the Inline Toolbar appearance

When editor is placed in a popup, the Inline Toolbar could move out of visible area

image

Sometimes position is much far from selection:

image

Cause

  1. Right now we're trying to place Inline Toolbar at the center of selection. There is a complex logic with CSS transforms and JS computations.
  2. this.width computation works incorrect since move() called before open() in where tools are added.

Solution

  1. addToolsFiltered() now called before move()
  2. I've simplified position calculation: now it will be aligned by the left side of selection. CSS transforms removed.

image

  1. If Inline Toolbar right coord does not fit text column, it will be opened right-aligned

image

  1. (Bonus) Debounce added to the Selection Change event. Now Inline Toolbar will be opened only when selection changing finished.
  2. (Bonus) Build modes added to yarn build command. Now we can detect build for tests and add some attributes like data-cy for elements querying. It's a better practice then querying by CSS class name.

@neSpecc neSpecc merged commit c5854ee into next Dec 8, 2023
6 checks passed
@neSpecc neSpecc deleted the fix/it-appear branch December 8, 2023 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants