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

Shouldn't link button be on when caret is inside a link? #4822

Closed
Reinmar opened this issue Feb 27, 2018 · 12 comments · Fixed by ckeditor/ckeditor5-link#201
Closed

Shouldn't link button be on when caret is inside a link? #4822

Reinmar opened this issue Feb 27, 2018 · 12 comments · Fixed by ckeditor/ckeditor5-link#201
Assignees
Labels
package:link status:discussion type:improvement This issue reports a possible enhancement of an existing feature.
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Feb 27, 2018

Right now the toolbar doesn't react to moving the caret which I suddenly found a bit odd. Bold, italic, block quote, etc. – they all are highlighted, but the link isn't.

This gets more important now when we add https://github.com/ckeditor/ckeditor5-link/issues/72.

@Reinmar
Copy link
Member Author

Reinmar commented Feb 27, 2018

cc @oleq @dkonopka @jodator

@jodator
Copy link
Contributor

jodator commented Feb 27, 2018

From my POV it could be highlighted as you described. It does look odd to me. Also for the two-step caret movement would be additionally good to highlight the link button as the caret goes into link.

@oleq
Copy link
Member

oleq commented Feb 27, 2018

If the button is "on" in this situation, then it supposed to be a 2-state.

What would happen if a user clicked the button in such situation? My common sense tells me the button goes off but... then what? Unlink?

And if it does not go off, how to tell the user that it was pushed (even further?) to open the link UI? It can't be "super-on" ;-)

@Reinmar
Copy link
Member Author

Reinmar commented Feb 27, 2018

What would happen if a user clicked the button in such situation? My common sense tells me the button goes off but... then what? Unlink?

Can't it simply focus the URL input (if it wasn't focused)? Sounds ok to me, but I'd need to see this live.

@oleq
Copy link
Member

oleq commented Mar 1, 2018

ATM the first step is the link actions view so there's no input there to focus. We could switch to the link editing view in such situation but I'm not sure this is the expected UX for the users.

@Reinmar
Copy link
Member Author

Reinmar commented Mar 7, 2018

I've been playing today with the link highlight and two-step caret movement and I find this really missing that nothing changes in the toolbar when the highlight appears. The button switching on would make the signal that you enter a link clearer.

Are you sure, @oleq, that it's a big deal that the button would be on?

@oleq
Copy link
Member

oleq commented Mar 8, 2018

Are you sure, @oleq, that it's a big deal that the button would be on?

Google Docs keeps it off (but they always display the balloon)

image

I also checked other WYSIWYG editors. Some follow this approach and some don't.

If this a such a big deal, then OK, let's have it on and see if this feels right.

@Reinmar
Copy link
Member Author

Reinmar commented Mar 8, 2018

Google Docs thinks that a link is underlined blue text :D So...

@Reinmar
Copy link
Member Author

Reinmar commented Mar 9, 2018

I've just reported https://github.com/ckeditor/ckeditor5-highlight/issues/16 which might be an answer to my question:

Are you sure, @oleq, that it's a big deal that the button would be on?

When the button is on I was expecting that pressing it will have an action. I'm not sure whether moving focus to link editing balloon will be enough. Perhaps it will, but we certainly have to provide that action.

@Reinmar
Copy link
Member Author

Reinmar commented Jun 18, 2018

Are you typing with a link or not?

image

Which also made me think that

	--ck-color-link-selected-background: 						hsl(201, 100%, 96%);

Should use a hsla() so to look better on non-white backgrounds. cc @dkonopka

@dkonopka
Copy link
Contributor

--- a/theme/ckeditor5-ui/globals/_colors.css
+++ b/theme/ckeditor5-ui/globals/_colors.css
@@ -94,5 +94,5 @@
        /* -- Link ----------------------------------------------------------------- */
-       --ck-color-link-selected-background: hsl(201, 100%, 96%);
+       --ck-color-link-selected-background: hsla(201, 100%, 85%, 0.4);

👍 Alpha channel is neccessary here, we can change hsl format to hsla globally in this variable without any doubts.

hsla() format inside table

screen shot 2018-06-18 at 11 26 00

hsla() format inside white content

screen shot 2018-06-18 at 11 26 07

@Reinmar
Copy link
Member Author

Reinmar commented Sep 5, 2018

Based on the first 3 comments I can see we have 5 votes for and @oleq's "ok".

I again worked on some things in CKEditor 5 and felt that it's missing. Plus, we had another slightly confused user asking how to write after links. Let's change the button's behaviour and see what happens.

Reinmar referenced this issue in ckeditor/ckeditor5-link Sep 7, 2018
Other: The link button will be active when the selection is placed inside a link. Closes #173.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-link Oct 9, 2019
@mlewand mlewand added this to the iteration 20 milestone Oct 9, 2019
@mlewand mlewand added status:discussion type:improvement This issue reports a possible enhancement of an existing feature. package:link labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:link 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