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

Link styles #43

Closed
wants to merge 2 commits into from
Closed

Link styles #43

wants to merge 2 commits into from

Conversation

Arty2
Copy link

@Arty2 Arty2 commented Aug 14, 2015

Current link styles use border-bottom , but the line is in the middle of the leading, and thus becomes obtrusive.

This pull requests is based on a technique of styling the links using CSS gradients and text-shadow.

This solution however requires additional selection styles and limits the colors that may be used for the background.

Also added hover styles to better inform the viewer that the link is clickable.

Visual improvement of underlines; hover interactivity.
@daveliepmann
Copy link
Contributor

Cool stuff! A couple questions.

  • I think it's a little too tight on Safari. Can you fix that?
  • It looks good to me on my laptop with FF and Chrome. Has it been tested on mobile devices?
  • Could you please put hover styles in a separate PR for discussion?

@Arty2
Copy link
Author

Arty2 commented Aug 14, 2015

Probably the background-position % needs some fine tuning but unfortunately I don't have access to Safari in order to do that.
Yes, have tested it on FF and Chrome for Android.
Sure thing; what would be the better way to do that, remove them from this branch and add another?

@daveliepmann
Copy link
Contributor

I can take a look at Safari and mobile Safari once it's merged.

A separate branch with a separate PR would be perfect for the hover styles. I'd like some discussion on whether A) they're a good idea regardless of color and B) what the ideal color is. I'm absolutely open to changing it but I'd like some community and expert input.

Visual improvement of underlines.
@Arty2 Arty2 mentioned this pull request Aug 14, 2015
@daveliepmann
Copy link
Contributor

Thanks for the quick revision. This is really solid work. Thank you.

I am going to think on this one a little while. Right now, to me, the underline looks better even though it interferes with the leading. The text-shadow dodging the descenders just grabs my eye in a bad way. And though I understand this may seem like a non-issue, it also looks weird when highlighting text, which a lot of people do compulsively.

More concretely, I'm concerned about the performance implications of text-shadow on long, link-heavy documents. The MDN notes that "Opera supports a maximum of 6-9 text-shadows for performance reasons", and I've seen multiple reports elsewhere.

@daveliepmann
Copy link
Contributor

(Regarding the performance concerns, they may only be a problem when rendering dynamically, which we don't do. I'm still researching the problem. Empirical tests using documents with lots of links could help.)

@Arty2
Copy link
Author

Arty2 commented Aug 14, 2015

Opera has long since switched to rendering with webkit so desktop users should be fine, however I just tested with Opera Mini for Android and the underline is not visible. User adoption of Opera Mini used to be high some years ago, but then again, few things on the modern web work on it anymore.

Reportedly Safari 8.0 already renders text-underline as such, but it’s eventually a matter of preference, so I can't argue more for them. :)

Performance with many too many links may be indeed an issue, and the publishers of the technique, mentioned in the first post, write:

On mobile and older browsers, it may have minor performance penalties. In our testing, we didn’t find much if any impact, especially since the text-shadows don’t use any spread value. But since you’re drawing new things that didn’t used to be there, it’s worth noting.

@crmackay
Copy link
Contributor

I like the effort, but it's a little wonky at times like here with a j in chrome:

image
image

I think the desired effect looks great, which would be an underline like this:

But, I also wonder if there isn't a better way to style links than with underlines:

Some examples: http://tympanus.net/Development/InlineAnchorStyles/

@daveliepmann
Copy link
Contributor

Hey, we ended up doing this independently, which makes this PR superfluous. I still appreciate your contribution, so thanks. I'll be keeping an eye on performance issues, but I doubt it'll be an issue for most docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants