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

Should dropdown items work as 2–state or simple apply? #2453

Closed
oleq opened this issue Jul 4, 2017 · 9 comments · Fixed by ckeditor/ckeditor5-heading#84
Closed

Should dropdown items work as 2–state or simple apply? #2453

oleq opened this issue Jul 4, 2017 · 9 comments · Fixed by ckeditor/ckeditor5-heading#84
Assignees
Labels
package:heading type:improvement This issue reports a possible enhancement of an existing feature.
Milestone

Comments

@oleq
Copy link
Member

oleq commented Jul 4, 2017

ATM the drop-down items work like 2–state buttons, i.e. bold:
kapture 2017-07-04 at 14 57 31
despite the "Paragraph" item, which reverts block to default <p>.

However, as @mlewand pointed out, MS Word and Google Docs implement such items as 1–state (1–way) buttons. They apply the block/format/style/font and never revert it when used multiple times.

It turned out in a F2F talk with @mlewand that users actually got used to this 1–way approach. They often have mixed styling in the content and they select a huge chunk of text and use a dropdown (like the heading) to make sure the styling is exactly what they need. ATM, they need to use the heading drop–down twice to do this (first click to <p>, then another to select the block), which is pretty annoying, I guess.

I think we should be consistent with major desktop text processors and with v4 in this matter (it's a cross–version issue, actually). WDYT? (cc @Reinmar, @fredck).

Maybe we should also start a discussion in http://ckeditor.github.io/editor-recommendations/?

@Reinmar
Copy link
Member

Reinmar commented Jul 4, 2017

I have no strong opinion about this. Especially when talking about the heading dropdown because it's very different from how font size, family, color or other styles are used. It will be extremely uncommon for people to try to select 10 lines of text which may contain headings already and apply some heading to all that.

It's a significantly different story with styling options. In this case you may have selection like this:

<p>foo<span color=red>bar[bom</span>boo]</p>

And you may want to apply the red color to all that. The toggle behaviour which CKEditor 4 implements at the moment will remove the color from "bom" which is the opposite of what the user would wants to do.

I guess there would be no harm if the heading dropdown worked like this as well, but I'm not sure if there's any harm from how it works like now.

@mlewand
Copy link
Contributor

mlewand commented Jul 4, 2017

I agree on case of spans. Talking about paragraphs/headings, as long as options are mutually exclusive, the behavior should be common in both cases.

Important question here is how we want to keep the possibility to remove given change. I think that one way would be to add "Default" (or similar sounding) option as a first one.

@fredck
Copy link
Contributor

fredck commented Jul 4, 2017

I find the current behavior wrong for a select box. It is definitely not a toggle button.

@fredck
Copy link
Contributor

fredck commented Jul 4, 2017

Maybe we should also start a discussion in http://ckeditor.github.io/editor-recommendations/?

ER is not about that. This is a pure UI issue.

@Reinmar Reinmar assigned Reinmar and unassigned Reinmar Jul 4, 2017
@Reinmar
Copy link
Member

Reinmar commented Jul 4, 2017

I find the current behavior wrong for a select box. It is definitely not a toggle button.

Good point.

Take a look at select boxes in GH's issue page. You have the multi-option ones like "Assignees" where you have toggle options (click your name twice and it will add and then remove you). But if you have single-option select boxes (like "Milestone"), then clicking an active option will not remove it.

I think it's a good rule.

@fredck
Copy link
Contributor

fredck commented Jul 4, 2017

True... it reflects the behavior of single vs multi option selection boxes in HTML.

@mlewand
Copy link
Contributor

mlewand commented Jul 4, 2017

What about restoring default value? GH automatically puts a distinguishable "Clear xyz" option at the beginning.

Typical Word processor doesn't need such option because it always holds full content. For our editor the case is different, as we often time are authoring part of the content, which might (and should) inherit formatting dictated by the layout.

@oleq
Copy link
Member Author

oleq commented Jul 4, 2017

Yeah, that's a good point @mlewand. We must allow users restore the default styling/heading/whatever because the content is often styled by web page CSS.

Maybe it's not that visible in heading feature because we already offer the "Paragraph" button in the dropdown but what about other features? I'm mostly talking in the context of v4 which produces lots of styling but maybe we'll face the same problem in the future in v5 as well?

@Reinmar
Copy link
Member

Reinmar commented Jul 4, 2017

Maybe it's not that visible in heading feature because we already offer the "Paragraph" button in the dropdown but what about other features? I'm mostly talking in the context of v4 which produces lots of styling but maybe we'll face the same problem in the future in v5 as well?

I think we'll need to decide per case. Here we have "Paragraph" and in other cases we'll have "Clear" or "Reset" or "Default".

@oleq oleq self-assigned this Jul 18, 2017
oleq referenced this issue in ckeditor/ckeditor5-heading Jul 18, 2017
Reinmar referenced this issue in ckeditor/ckeditor5-heading Jul 23, 2017
Other: Heading dropdown items should never revert the state, apply only. Closes #83.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-heading Oct 9, 2019
@mlewand mlewand added this to the iteration 11 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:improvement This issue reports a possible enhancement of an existing feature. package:heading labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:heading 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.

4 participants