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

Using a '-' divider in toolbar items config without the shouldNotGroupWhenFull option enabled causes displaying an unwanted separator. #8582

Closed
pkwasnik opened this issue Dec 3, 2020 · 5 comments · Fixed by #8656
Assignees
Labels
domain:ui/ux This issue reports a problem related to UI or UX. intro Good first ticket. package:ui squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@pkwasnik
Copy link
Contributor

pkwasnik commented Dec 3, 2020

📝 Provide detailed reproduction steps (if any)

  1. In the editor configuration inside items list set - divider on any position
  2. shouldNotGroupWhenFull should be set to false (it is the default option) 
  3. Open editor

✔️ Expected result

There should not be any separator inserted in the place of -. Using - without shouldNotGroupWhenFull should not affect the toolbar.

❌ Actual result

There is an unwanted separator inserted in the place of -.

📃 Other details

  • Browser: …
  • OS: …
  • CKEditor version: …
  • Installed CKEditor plugins: …

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@pkwasnik pkwasnik added type:bug This issue reports a buggy (incorrect) behavior. package:ui domain:ui/ux This issue reports a problem related to UI or UX. labels Dec 3, 2020
@Reinmar Reinmar added this to the nice-to-have milestone Dec 7, 2020
@Reinmar
Copy link
Member

Reinmar commented Dec 7, 2020

Haha, is this a feature or a bug? :D

We talked about logging a warning that shouldNotGroupWhenFull is enabled and a separator is used. This may be the simplest solution.

@Reinmar Reinmar added the squad:core Issue to be handled by the Core team. label Dec 7, 2020
@oleq
Copy link
Member

oleq commented Dec 7, 2020

We talked about logging a warning that shouldNotGroupWhenFull is enabled and a separator is used. This may be the simplest solution.

There is one. But maybe we should not render the view at all in this case.

@oleq oleq added the intro Good first ticket. label Dec 7, 2020
@Reinmar
Copy link
Member

Reinmar commented Dec 11, 2020

But maybe we should not render the view at all in this case

Which view? I don't understand what's the expected behavior in this case.

@oleq
Copy link
Member

oleq commented Dec 11, 2020

  1. - should create a new toolbar line, as in the documentation.
  2. If, due to other configuration (shouldNotGroupWhenFull === false) this is not possible, - should do nothing.
  3. According to the bug report, it spreads toolbar items, which is against 1.
  4. This means the behavior described in the bug report is indeed a bug.
  5. The UI View corresponding to - in the toolbar configuration should not be rendered at all to address this bug.
  6. In other words: we should not only warn the user in the console but also get rid of the goofy View from the toolbar when this happens.

@Reinmar
Copy link
Member

Reinmar commented Dec 11, 2020

Thanks. That clarifies it.

Reinmar added a commit that referenced this issue Dec 15, 2020
Fix (ui): A '-' (new line) divider will not be rendered when grouping is enabled. Closes #8582.
@Reinmar Reinmar modified the milestones: nice-to-have, iteration 39 Dec 15, 2020
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. intro Good first ticket. package:ui squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants