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

CSS highlighting in SICP maths book #220

Closed
danielweck opened this issue Oct 3, 2018 · 8 comments
Closed

CSS highlighting in SICP maths book #220

danielweck opened this issue Oct 3, 2018 · 8 comments

Comments

@danielweck
Copy link
Member

Issue originally reported here:
sarabander/sicp#29 (comment)

@naorunaoru said:

R2 Reader on iOS works mostly fine — SVG, MathML seem to be working. Code snippets are rendered in monowidth typeface, but with no highlighting. That'll do.

Test EPUB
https://github.com/sarabander/sicp-epub/blob/master/sicp.epub
(see https://github.com/sarabander/sicp )

@danielweck
Copy link
Member Author

Related issue: readium/r2-testapp-kotlin#112

@JayPanoz
Copy link

JayPanoz commented Oct 4, 2018

Hi, ReadiumCSS’ designer there.

So I took a quick look this morning and indeed, code isn’t highlighted in night mode (should be in day and sepia though).

Reference screenshots:

photo 04-10-2018 14 27 20

photo 04-10-2018 14 27 40

photo 04-10-2018 14 27 53

Actually, the absence of code highlighting is to be expected in the r2-swift-testapp, which is using the default Readium-CSS as night mode is an exception. There’s even a closed issue explaining possible issues, complications, etc.: readium/readium-css#7

Overriding color was an accessibility- and interoperability-driven choice. To put it simply:

  1. there was no way to guarantee sufficient contrast e.g. some darker colors would have a very low contrast on a black background, a lot of colors have a low contrast as well on a gray background (similar to the one iBooks offers for instance, reading apps using Readium-CSS can indeed do that as Readium-CSS is a reference);
  2. there is also a possibility of colors being too bright (too much contrast), that can escalate to visual vibration;
  3. that led to interoperability in some way as we therefore decided to be consistent with other well-known apps, including iBooks.

Note we are very aligned with iBooks there, especially:

• Will override color but not -webkit-text-fill-color.

Which is quite an old CSS trick people have been using to force some colors in iBooks. But once again, I can’t guarantee one or several reading apps won’t create other reading modes (mint, dark blue, dark gray, etc.) in which those forced colors will offer users a very low contrast, making the code snippets very uncomfortable to read.

@naorunaoru
Copy link

naorunaoru commented Oct 15, 2018

Fair enough. Though there actually might be some way to circumvent this, for example, we can calculate luminance difference between background and text color and if it's lower than some user-selectable threshold than increase text luminance. iTerm2 does something like this with its Minimum contrast slider in profile preferences.

Or we can even invert text color (that is defined by color or -webkit-text-color) when background luminance is low. Some apps do this already, can't say for sure, but Kobo may be one of them.

@JayPanoz
Copy link

Yeah that’s an option but that was out of scope for the ReadiumCSS project: the goal was indeed to have every module constrained to CSS.

As a disclaimer, we discussed options a lot during the engineering calls and it was agreed to avoid traversing the DOM (with JS) whenever possible, because that can be very costly in terms of performance.

Implementers can however add/remove modules and use some JS logic if they want. Note there’s also another CSS solution, using filter, cf. https://gist.github.com/JayPanoz/e9aac931795a152b0f2eb8494c6ead60 but it has its share of drawbacks too.

@naorunaoru
Copy link

Thanks for explanation! DOM traversal is slow as hell for sure.

@HadrienGardeur
Copy link

Do we need to keep this issue opened?

@mickael-menu
Copy link
Member

It doesn't seem like we can fix this directly in the Swift projects. Let me know if you want to transfer this issue to readium-css @JayPanoz

@JayPanoz
Copy link

JayPanoz commented Feb 8, 2021

@mickael-menu

As far as I can tell after re-reading the issue, there’s not much we can do in Readium CSS either. Even with some hack-ish use of filters, etc. we would get suboptimal results – and that’d be in ideal conditions with proper semantic markup.

@mickael-menu mickael-menu transferred this issue from readium/r2-testapp-swift Aug 12, 2022
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

No branches or pull requests

5 participants