Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Enlarged link's highlight background. #160

Merged
merged 2 commits into from
Mar 12, 2018
Merged

Enlarged link's highlight background. #160

merged 2 commits into from
Mar 12, 2018

Conversation

dkonopka
Copy link
Contributor

@dkonopka dkonopka commented Mar 12, 2018

Suggested merge commit message (convention)

Other: Enlarged background in .ck-link_selected. Closes ckeditor/ckeditor5#3404.

@dkonopka dkonopka requested a review from oleq March 12, 2018 11:19
outline: 1px solid var(--ck-color-link-selected-background);

/* #155. Let's keep the same background inside Highlight feature. */
& mark {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't hardcode something like this. We don't even know whether highlight will produce a <mark> element. Is there a different solution?

Copy link
Contributor

@jodator jodator Mar 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Reinmar AFAIR this feature is strictly <mark>-only. I don't recall an issue/coment but the outcome was that for highlight feature we provide <mark> always (no way to override).

Copy link
Contributor Author

@dkonopka dkonopka Mar 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Reinmar We can try .ck-link_selected > * but it can be buggy in the future (quick example: with user markers or comments in collaboration?).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is that it's not going to work anyway if we'll have a font color plugin which will set inline styles.

I need to see this live, but I think that perhaps we might just live with markers overriding link's highlight. That's still an acceptable solution to me.

@Reinmar
Copy link
Member

Reinmar commented Mar 12, 2018

image

Turns out that the proposed style doesn't help – see the text color being reset in part of the link? That's exactly what I'm worried about – bulletproofing this solution is trivial.

That's why I'd rather prefer this:

image

Let's see if anyone will complain. Looking at this now, I'd say that this is even perhaps what people will expect.

@Reinmar
Copy link
Member

Reinmar commented Mar 12, 2018

mar-12-2018 16-53-40

@Reinmar Reinmar merged commit a7f1925 into master Mar 12, 2018
@Reinmar Reinmar deleted the t/155 branch March 12, 2018 15:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enlarge link's highlight background
3 participants