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

Block toolbar in Balloon Block Editor build doesn't group items #6449

Closed
panr opened this issue Mar 16, 2020 · 2 comments · Fixed by ckeditor/ckeditor5-ui#554
Closed

Block toolbar in Balloon Block Editor build doesn't group items #6449

panr opened this issue Mar 16, 2020 · 2 comments · Fixed by ckeditor/ckeditor5-ui#554
Assignees
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:build-balloon-block type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@panr
Copy link
Contributor

panr commented Mar 16, 2020

Provide a description of the task

While fixing the issue related to toolbar grouping in Balloon and Inline Editor I forgot to add support for this to balloon block toolbar, so in the Balloon Block Editor build (https://ckeditor.com/docs/ckeditor5/latest/examples/builds/balloon-block-editor.html) we have something like this in the smaller viewports.

image

In this case, the best solution (probably) is to bind the maxWidth value to the window width, not the editable as we did in balloon and inline toolbar.

📃 Other details

Issue is related to #5597

@panr panr added type:task This issue reports a chore (non-production change) and other types of "todos". domain:ui/ux This issue reports a problem related to UI or UX. package:build-balloon-block labels Mar 16, 2020
@mlewand mlewand added this to the iteration 31 milestone Apr 6, 2020
@panr
Copy link
Contributor Author

panr commented Apr 6, 2020

In this case, the best solution (probably) is to bind the maxWidth value to the window width, not the editable as we did in balloon and inline toolbar.

Before I start working on this, could you @oleq confirm if ☝🏻this is the right direction?

@oleq
Copy link
Member

oleq commented Apr 7, 2020

In this case, the best solution (probably) is to bind the maxWidth value to the window width, not the editable as we did in balloon and inline toolbar.

Not, really. If the max-width of the toolbar equalled the viewport width, then because the toolbar is attached to the button some (unknown) distance from the left (or right in RTL) boundary of the viewport, the toolbar will almost always stick out the viewport (even if we use a "safe 90% factor").

IMO in this case

image

  • the right edge of the toolbar should never go farther than the right edge of editable (this is where max-width should trim the toolbar)
    • IMO that may be a tricky thing to do because:
      • we don't know where the block button is (it could be 10px or it could be 100px from the editable)
      • the positioning of the toolbar is not simple (it sticks out a little to the left)
    • so to make it simple, it would be best if we could do all the math after the toolbar showed up and has been positioned, then all we need to do is toolbar.maxWidth = toolbar.width - ( toolbarRect.right - editableRect.right )
  • the toolbar should be attached east of the button (a followup issue BTW)

oleq added a commit to ckeditor/ckeditor5-ui that referenced this issue Apr 9, 2020
Feature: The `BlockToolbar` should group items when there is no place to show them all. Closes ckeditor/ckeditor5#6449. Closes ckeditor/ckeditor5#6575. Closes ckeditor/ckeditor5#6570.

Bulletproofed the `ToolbarView#maxWidth` and items grouping when the toolbar is invisible.
mlewand pushed a commit that referenced this issue May 1, 2020
Feature: The `BlockToolbar` should group items when there is no place to show them all. Closes #6449. Closes #6575. Closes #6570.

Bulletproofed the `ToolbarView#maxWidth` and items grouping when the toolbar is invisible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:build-balloon-block type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants