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

Improve link styling for high-contrast theme #885

Closed
wants to merge 1 commit into from
Closed

Improve link styling for high-contrast theme #885

wants to merge 1 commit into from

Conversation

The-Compiler
Copy link
Contributor

Looking at #84, the high-contrast theme originally was created for eReader
devices.

On my device (an Onyx Boox Nova Pro), it works quite well, but both links (being
a rather dark blue) and normal text get rendered as black, making it impossible
to tell them apart from normal text.

This change makes links black and re-adds the underlining.

cc @shtrom

Looking at #84, the high-contrast theme originally was created for eReader
devices.

On my device (an Onyx Boox Nova Pro), it works quite well, but both links (being
a rather dark blue) and normal text get rendered as black, making it impossible
to tell them apart from normal text.

This change makes links black and re-adds the underlining.
@di72nn
Copy link
Member

di72nn commented Dec 30, 2019

We currently have a high-contrast and an "e-ink" theme, which only differ in that the "e-ink" one has font-weight set.
I think it would be better to modify only the "e-ink" theme (since I have no idea what preferences other users of the high-contrast theme have), would it be ok with you?

If so, I would suggest to simply rename weighted-font style to something like e-ink (in CSS and here) and add your changes with that style.

Update: Now I can see you suggested renaming weighted-font yourself, I missed it.

@The-Compiler
Copy link
Contributor Author

The-Compiler commented Dec 30, 2019

Yeah, I wasn't sure about it - not sure who would use the high contrast theme without an e-ink device. If we're thinking of e.g. people with bad vision, I guess it'd make sense to have the link text in black (to have a high contrast) as well.

Some related discussions:

For high contrast themes in light and dark i would keep this bold and underlined, because color would decrease the contrast.

Isn't the black theme also used on regular Android devices as an "energy saving" theme for AMOLED devices ?

Also pinging @tenkabuto @di72nn (whoops, that's you!) @silberzwiebel from there in case they have some more insights.

I personally don't care much either way - let me know how to proceed and I will 😉

@tcitworld
Copy link
Member

I agree with suggestions from @di72nn. Let's at least make the weighted-font rename.

@silberzwiebel
Copy link
Contributor

I'm using the e-ink theme right now on an inkbook prime and can tell apart links from text just fine. Links are light-grey, text is black.

@The-Compiler
Copy link
Contributor Author

Hmm, okay, seems like this is only needed for Onyx devices then. After thinking about it some more, I'd propose this:

  • Rename weighted-font to e-ink anyways (just because it makes things more clear)
  • Try working around Onyx' stupid style injection (so that links are grey instead of black)
  • If that's impossible, add a new "underline links" options to the settings, so this can be turned on independently from the theme and only for e-ink devices where this is needed.

How does that sound? Closing this in the meantime. I'll try to follow up with new PRs for these, but it might take a while, as I have quite a bit on my plate right now.

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 this pull request may close these issues.

6 participants