Skip to content

Conversation

@ddfreiling
Copy link
Contributor

Optionally set decorations at z-index: -1, in order to support opaque highlights behind text, but in front of background-color. This ensures that highlights do not blur/modify text color via alpha overlay.

From discussion with @mickael-menu I named this experimentalDecorationPositioning and turned it off by default. However I do think it's a superior solution and experience for the visually impaired user. I plan to add this feature to kotlin-toolkit as well.

Without experimental positioning:
without_exp_1
without_exp_2

With experimental positioning:
with_exp_1
with_exp_2

optionally set decorations at z-index: -1, in order to support opaque highlights behind text and not blur/modify text color via alpha overlays
Copy link
Member

@mickael-menu mickael-menu left a comment

Choose a reason for hiding this comment

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

Thank you @ddfreiling, this is promising!

I changed the Configuration constructor and kept only the experimentalPositioning in the HTMLDecoration.defaultTemplates() API. If we don't encounter any issue in production, this will become the new default. 👍

@mickael-menu mickael-menu requested a review from Copilot November 6, 2025 10:41
@mickael-menu mickael-menu changed the title feat: add experimental decoration positioning Add experimental EPUB decoration positioning Nov 6, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces experimental positioning for EPUB decorations, allowing highlights to be placed behind text to improve legibility when using opaque decorations (alpha: 1.0).

Key changes:

  • Added experimentalPositioning parameter to HTMLDecorationTemplate.defaultTemplates() and related factory methods
  • When enabled, decorations are positioned with z-index: -1 to place them behind text
  • Updated the TestApp to demonstrate the feature with opaque highlights

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
Sources/Navigator/EPUB/HTMLDecorationTemplate.swift Added experimentalPositioning parameter to template factory methods and CSS z-index styling
TestApp/Sources/Reader/EPUB/EPUBViewController.swift Updated to use opaque highlights with experimental positioning enabled
CHANGELOG.md Documented the new experimental positioning feature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mickael-menu mickael-menu merged commit 7d904ee into readium:develop Nov 6, 2025
5 checks passed
@mickael-menu
Copy link
Member

@ddfreiling By the way I noticed an issue already, if we set a light foreground color in the preferences:
Screenshot 2025-11-06 at 12 01 34

We may need a way to provide a map of colors according to the provided theme. But maybe it's enough to let the app refresh the decorations directly.

@ddfreiling
Copy link
Contributor Author

@ddfreiling By the way I noticed an issue already, if we set a light foreground color in the preferences:

Screenshot 2025-11-06 at 12 01 34

We may need a way to provide a map of colors according to the provided theme. But maybe it's enough to let the app refresh the decorations directly.

Isn't this just because the alpha is also set to 1.0 in the example app now to highlight the difference? Without the position fix using alpha 1.0 would just completely obscure the text

@mickael-menu
Copy link
Member

Yes, if you reset the alpha to 0.3 it remains as readable as before. But that's a shame if we can't use a 1.0 alpha in light mode at the same time.

darktasevski pushed a commit to darktasevski/swift-toolkit that referenced this pull request Nov 7, 2025
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.

2 participants