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

Enlarge link's highlight background #3404

Closed
Reinmar opened this issue Mar 7, 2018 · 11 comments · Fixed by ckeditor/ckeditor5-theme-lark#160
Closed

Enlarge link's highlight background #3404

Reinmar opened this issue Mar 7, 2018 · 11 comments · Fixed by ckeditor/ckeditor5-theme-lark#160
Assignees

Comments

@Reinmar
Copy link
Member

Reinmar commented Mar 7, 2018

Current:

image

With 1px outline:

image

WDYT?

@Reinmar
Copy link
Member Author

Reinmar commented Mar 7, 2018

cc @oleq @dkonopka

@oleq
Copy link
Member

oleq commented Mar 8, 2018

LGTM. Is this some padding or an actual outline?

@dkonopka
Copy link
Contributor

dkonopka commented Mar 8, 2018

Looks good to me too, it's padding I guess?

@Reinmar
Copy link
Member Author

Reinmar commented Mar 8, 2018

No, no padding. Padding is dangerous in inline stuff. Just a 1px outline.

@Reinmar
Copy link
Member Author

Reinmar commented Mar 8, 2018

We could perhaps even play with the colour of this outline. I wonder if it would look any good if it was e.g. slightly darker, to create a kind of a border. Something which will indicate the integrity of this link (not sure how to tell this).

@oleq
Copy link
Member

oleq commented Mar 8, 2018

Some ideas:

screen shot 2018-03-08 at 16 22 54

screen shot 2018-03-08 at 16 22 47

screen shot 2018-03-08 at 16 22 40

@Reinmar
Copy link
Member Author

Reinmar commented Mar 8, 2018

Hm... From all these, I'd choose the 1st option (solid). But at the same time, I can see that we don't have enough space inside that box, so it gets too crowdy in there. So, I'd prefer the same colour as the bg.

@dkonopka
Copy link
Contributor

dkonopka commented Mar 9, 2018

I'm for keeping it simple - outline with the same background looks 👍

@oleq
Copy link
Member

oleq commented Mar 12, 2018

@dkonopka Can you create a PR?

@dkonopka
Copy link
Contributor

dkonopka commented Mar 12, 2018

Guys, what do you think in the situation with highlight feature?
Should .ck-link_selected change background on children elements like <mark>?

Current

link-current

Proposal with outline

link-in-highlight

Proposal with outline & background

highlight-in-link-proposal

@Reinmar
Copy link
Member Author

Reinmar commented Mar 12, 2018

Proposal with outline & background

👍

Link highlight, just like selection's highlight, should have the precedence.

Reinmar referenced this issue in ckeditor/ckeditor5-theme-lark Mar 12, 2018
Other: Enlarged background in `.ck-link_selected`. Closes #155.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-theme-lark Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants