-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Highlight UI & UX #2595
Comments
I like the third option - the one without text - the most with fourth/fifth as second. Maybe if marker/pen icons could be a bit more distinct from each other? Just a though as currently they are pretty similar (especially when scaled down a bit). Also marker tip in icon has narrower tip then in pen icon. |
I actually wanted to comment that I have no idea what these icons mean. I can only guess that one of them sets the text color, the other bg color, but I don't know what the 3rd option does. EDIT: After having 3rd look at this I noticed the label there :D It makes it more clear now and perhaps will be sufficient for the users. But I'd still agree with @jodator that icons could be more distinctive. |
I had a dream once about this feature. I would love to see how that would look like :) |
Btw, we can also think well about the list of colors we want to configure by default. |
I don't think that more than four colors is necessary for the scope of the feature. This is configurable, after all. |
I'm for this:
|
@Reinmar or maybe from Crayola? 😉 |
Updated proposals then: Should "eraser" button remove background & color of highlight or we need separate buttons? In my opinion, we should stay with one (KISS). Another question: what is more important if color&background is enabled, which icon should we show? Both as proposed? Color & background highlight enabled: |
@dkonopka no mixing (for sure in MVP) so it's either background or color at once. |
Let's get back to the starting point here to not mess things up. We wanted to propose this feature based on a specific use case: users want to mark parts of the content with some color (usually as a review tool). Let's pay attention to not transform this feature into the old "Foreground and Background Color" feature. It's about "highlight" and one can do so either by using a marker or by writing in a different color. That's our goal. |
I believe that the option I click in the panel should be the one I see in the toolbar after that. I mean, the panel should list icons that are also used as the toolbar button icon. |
Btw, "Yellow Marker" seems to be the most common option for this (it's, in fact, the way browsers style |
I'm still not sure if the user will properly recognize marker and pen icon (as you @fredck suggested in #631 (comment) ). I was trying to put there even A/T letter to show typography, but it's not fit as good as I was expecting. Proposal 2&3 clearly represent a state of the upcoming action. In my opinion, it doesn't look like old-type background/foreground tool and there is no way to wrongly understand event "behind" icon. 1. Highlight background and Highlight color as pen&marker icon2. Highlight with common marker icon3. Highlight with common cropped marker icon |
Cropped icon looks better. I have only problem with too small contrast on those icons (2&3) - the "zigzag" icons were better in that matter as there was better contrast. |
Another idea is making the options in the panel different than the icon, but also bigger. That was my initial idea in the mockup I've put down on paper earlier. And sincerely, the proposed options don't look good to me. For now, the 3rd option proposed here is the one I like the most: https://github.com/ckeditor/ckeditor5-highlight/issues/3#issuecomment-344206677 |
I was thinking about it too, do we really need to "copy" icon from the main toolbar to options panel?
☝️ I really prefer "zigzags" more than "marker/pen" icons. I think the first proposal looks modern opposite to default "color-palette" in most apps. @oleq WDYT? 1. Inline "zigzags"2. "Zigzags" with label3. "Color-palette" style |
TBH, to me, the best option is "3. Highlight with common cropped marker icon" I just looked at it and I immediately knew what the feature is about, what to expect and how to use it. Ideas
IssuesAlthough I like the icons, please note that people may create blue/black/dark markers. They must be visible in the palette, which has a dark background and vice versa if someone used a bright theme for the editor. They must be bulletproof. What we need in the mockups:
It's all about contrast and basic usability and accessibility. We mustn't allow any icon to disappear in any situation. |
I like "Color-palette" style" very much. This clean and simple. I don't get what "highlight color" is. Is it text color? Can we call it "highlight color" and "font color" like MS Office do? |
I'll add this one to have more options as I asked @dkonopka to create this variant (but he didn't like it as much as zigzag ;) ). Probably the line on the bottom is to be removed in this version: |
@oleq With gaps it looks more separated and very clear. Like we talked about contrast and case with black/dark green color/marker, we will wait for community feedback. Anyway, if someone want so much black marker then it will be possible to change balloon background (or even icon?). I think is edge case when user will need dark marker similar to our balloon bg. |
So, do I understand correctly that the last proposed version is this one? I like its simplicity, but at the same time it worries me that these icons will not be understandable. I got confused by this feature initially. So, if we'd go this way, we'll need to work hard on proper icons. Also, labels will be important to get it right. But we can also address this issue (the lack of clarity) by choosing a more verbose version like this one: I really like it and it and so did @pjasiun. It's clear and simple and easier to extend. |
IMO it's a final design as we talked with @oleq. Ready for comments about details like icons/spacings, but yes, it's the last version of Highlight mockup. |
We're aware that these icons have strong accessibility issues, right? Even trying to push it with the "dark yellow" used in the design, the contrast with the white pen inside of it is not good. Imagine what would happen if we put the real yellow there. Additionally, a real-life marker is a pen with a colored tip... exactly what we use to show a "pen" in the proposed design, not the marker. |
@fredck that was a reason I've pushed this mockup (a dark marker/pen). But taking into a consideration that in split button should also represent last used marker (ie "yellow background") it shold be distinguishable between marker/pen variants. In other words the "more verbose" version is less verbose then that. In my opinion the zygzags are pretty close to:
|
Strongly agree on this. |
My 5cts. I think that the less "graphical" the icons, the better. I like most the version without any additional icon (3. "Color-palette" style), but zig-zags are okay too. Markers icon are too overwhelming, totally against the whole "lightweight" UI. Please, guys, don't go with marker-icons, they... don't look good. Trust me on this :P |
I really like proposed icons, they are fresh & modern. But in my opinion for the current toolbar it is not fitting as much as it could (for this icon set). If we are going to refresh UI for CK5 these new icons could look very good IMHO, for dark-theme too. |
I'd wait for the rest of the theme to catch up. These are really nice icons themselves but they don't match the rest of the icons because they are very graphical (the first one is even comic-like). |
To unblock @jodator I pushed the icons in ckeditor/ckeditor5-highlight@3523d5f. They might not be perfect but they do the trick. The inner color can be adjusted using CSS svg .ck-icon__fill { fill: yellow } I think the easiest way to do that at this moment is to retrieve |
@oleq: Some previous talks required one to define this color in configuration: editor.config.define( 'highlight', [
{
model: 'marker',
view: { name: 'mark', class: 'marker' },
title: 'Marker',
color: '#ffff66',
type: 'marker'
}
] ); The .marker {
color: #ffff66
} I'd like to clarify the solution (either):
|
The current config is too verbose but for a different reason. We don't need to define that it's the As for the color... I'm for defining it in the config. Retrieving it from the stylesheet by using the same class names will be really tricky IMO. Those styles would be then hard to write (e.g. you couldn't use the Finally, the option name should be |
@Reinmar: Another question: should highlight behave the same as basic commands on collapsed selection when not in highlight? Ie.: apply highlight to a collapsed selection and start typing - the text will be highlighted? |
I'm not 100% sure now. Do you remember @oleq what we decided? |
I'm not sure if we discussed this behavior, but I assume highlight is a post-creation tool. That would mean such an action (highlight collapsed selection, then type) should not be possible. But what about the UI then? Should the toolbar icon be disabled in a collapsed selection? |
But looks OK to me after more time fiddling with this. |
ps.: wouldn't be better to unselect highlighted text and create collapsed selection to range end? It is not visible ATM that selection get highlighted (nothing changes) as selection cover highlight. Other features' changes are visible under selection (like bold, italics, font-size, etc). |
What do you mean by that? Pressing CTRL+B does not change anything (visually) when the selection is collapsed. |
@oleq I've talking about other case on expanded selection (normal use-case). When you select some text and execute highlight - visually nothing happens. While executing bold the text get bolded and it is visible under a selection. Similar with italic. But for highlight you have to unselect the text to actually see command's result on text. |
Now I get it. Maybe we can tune the styles for (at least) markers so the native selection does not cover the entire highlighted area? |
Doesn't look too bad to me. I'd wait for more feedback, though. |
|
Sounds like a plan. As for the link–at–the–end–of–the–line issue, I'm leaving this for future reference: https://github.com/ckeditor/ckeditor5-link/issues/72#issuecomment-352458361. Can't wait to see it stable! |
Feature: Implemented the user interface of the highlight feature. Closes #3. Closes #4.
This is a follow up of #631.
In proposals below, you can see two types of presenting color attributes.
I'm definitely for v2 (color grid ) with a label because we need to explain users difference between "pen" and "markers", in my opinion, it's not clear to everyone (I mean icons). Version one is like the old-school style, so I'm not sure about it.
Additionally, we can use "Highlight color" & "Highlight background" to be different from old tools naming like "Font color/Text color/Font background".
The text was updated successfully, but these errors were encountered: