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

The arrow next to the "merge cells" button seems to be disabled #6631

Closed
Reinmar opened this issue Apr 17, 2020 · 9 comments · Fixed by ckeditor/ckeditor5-table#307
Closed
Assignees
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:table status:discussion type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@Reinmar
Copy link
Member

Reinmar commented Apr 17, 2020

📝 Provide a description of the improvement

When the button is disabled (because it's not applicable to single cell selections), the dropdown with options such as "split" or "merge cell left" should be available. It can be opened by click the arrow. However, to me, that arrow seems to be invisible. When the button is disabled, the arrow may be missed. I think that discoverability of that dropdown will be really hard.

I honestly don't know how we can improve this... Maybe the button shouldn't be off? Maybe it should have no action or display something when a single cell is selected?


If you'd like to see this improvement implemented, add a 👍 reaction to this post.

@Reinmar Reinmar added type:improvement This issue reports a possible enhancement of an existing feature. package:table domain:ui/ux This issue reports a problem related to UI or UX. labels Apr 17, 2020
@Reinmar
Copy link
Member Author

Reinmar commented Apr 17, 2020

cc @oleq @panr

@oleq
Copy link
Member

oleq commented Apr 20, 2020

I agree that it looks a little bit odd. Here are some options that popped into my head:

  1. Make the merge cells a standalone button. Then make the merge/split tools a regular drop-down (always enabled). Problems:
    • We need two different icons for the same functionality,
    • We're bloating the toolbar.
  2. We ignore the problem then implement the unmerge functionality bound to the main button. The button will always be enabled and the vast majority of users will never need to open the drop-down in the first place. Problems:
    • We need another feature.
  3. Somehow re-design of the split button to address the issue (???). Problems:
    • This will probably result in the re-design of the rest of the components.
    • That's pretty time-consuming.

@Reinmar
Copy link
Member Author

Reinmar commented Apr 20, 2020

I wonder if this could work:

  • When multiple cells are selected, the button merges them.
  • When only one, clicking the button opens the dropdown (button and arrow work in the same way).

At first I thought that it may be confusing. But then I realised that nothing bad will happen if you have just one cell selected – you'll simply see the dropdown panel with available options. When you have multiple cells selected, you will have a high chance of figuring out that there's a button and arrow and both are clickable. And if you click the button hoping for it to open the dropdown, you can always undo that.

@mlewand
Copy link
Contributor

mlewand commented Apr 20, 2020

  • When only one, clicking the button opens the dropdown (button and arrow work in the same way).

But you mean that the button would still have an icon of merge command? That would be misleading. How about changing the split button's "face" action in case the default one is disabled?

  1. Make the merge cells a standalone button.

Although this also seems to be a viable option for now - we don't have a whole lot of buttons in the dedicated table toolbar. So seems like a cheap solution to buy time for a proper fix.

Agree that currently anybody looking at it first will always think it's disabled. Personally never seen something like that before.

@oleq
Copy link
Member

oleq commented Apr 20, 2020

  • When multiple cells are selected, the button merges them.
  • When only one, clicking the button opens the dropdown (button and arrow work in the same way).

Not sure if this is possible with the current architecture. That could mean a brand new component for that purpose.

Also... I agree with @mlewand that if you click something that looks like a drop-down it should act like one. It should open the panel, not execute some editing action.


But you mean that the button would still have an icon of merge command? That would be misleading. How about changing the split button's "face" action in case the default one is disabled?

So like making the button disabled but announcing it by changing the icon only? This what the "disabled" state of the button is for. Also, users won't be able to tell why the thing is not working (the button is enabled, right?). And they will have to learn what each of the icons means, which could be a steep learning curve.


Agree that currently anybody looking at it first will always think it's disabled. Personally never seen something like that before.

There's a similar case somewhere in CF. I remember explaining how to do that to someone in their team.

@Reinmar
Copy link
Member Author

Reinmar commented Apr 20, 2020

Make the merge cells a standalone button. Then make the merge/split tools a regular drop-down (always enabled). Problems:

  • We need two different icons for the same functionality,
  • We're bloating the toolbar.

Not really an option to due to the problems that you describe. I can't imagine how the icons could look.

We ignore the problem then implement the unmerge functionality bound to the main button. The button will always be enabled and the vast majority of users will never need to open the drop-down in the first place. Problems:

  • We need another feature.

Unmerge will also sometimes need to be off (when that cell wasn't merged previously). Also, I feel that it would be even more confusing than my proposal as you may be surprised by the result.

Somehow re-design of the split button to address the issue (???). Problems:

  • This will probably result in the re-design of the rest of the components.
  • That's pretty time-consuming.

I can't imagine simple and local changes that could be done here. Redesigning the entire toolbar for that is a no go too. So if you're consider it time-consuming, I don't think it's an option.

:( 

I'm afraid that this is a quite critical issue (the current styles) and that the only option is to make this a dropdown button and move the "Merge cells" to the dropdown.

@niegowski
Copy link
Contributor

What if we split that dropdown in two separate buttons (merge and split) and get rid of "Merge cell up/left/right/bottom" (I think that this is legacy and very confusing). So we would have button for merging (without dropdown), and dropdown for split vertically/horizontally.

@Reinmar
Copy link
Member Author

Reinmar commented Apr 20, 2020

get rid of "Merge cell up/left/right/bottom" (I think that this is legacy and very confusing)

We need these options for environments (touch devices and for now visually impaired users) where multi-cell selections are not available.

As for whether it's a good idea assuming that in a longer perspective we'll resolve the issues with touch devices and lack of keyboard support, IDK. Perhaps that'd be the way to go then.

@Reinmar Reinmar added this to the iteration 31 milestone Apr 21, 2020
Reinmar added a commit to ckeditor/ckeditor5-table that referenced this issue Apr 21, 2020
Internal: The "Merge cells" split button should always be enabled to improve discoverability of tools in the dropdown. Closes ckeditor/ckeditor5#6631.
@jodator
Copy link
Contributor

jodator commented Apr 22, 2020

Unfortunately, the current behaviour look  broken to me. For some time I thought that something is broken.

mlewand pushed a commit that referenced this issue May 1, 2020
Internal: The "Merge cells" split button should always be enabled to improve discoverability of tools in the dropdown. Closes #6631.
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:table status:discussion type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants