Skip to content

Removal of advanced flag #168

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

Closed
JayPanoz opened this issue Mar 18, 2025 · 5 comments · Fixed by #170
Closed

Removal of advanced flag #168

JayPanoz opened this issue Mar 18, 2025 · 5 comments · Fixed by #170
Assignees
Labels
2024 Update New discussions, issues and requests triggered by the 2024 update

Comments

@JayPanoz
Copy link
Collaborator

The readium-advanced-on flag (and publisher’s settings toggle in various apps) was primarily created because of the font-size normalisation in ReadiumCSS v1 . Thanks to the rewritten fontSize submodule (using zoom), we don’t have to normalize anymore.

Now lots of settings depend on this flag:

  • ligatures;
  • hyphens;
  • letter-spacing;
  • line-height;
  • paragraph spacing;
  • disabling ruby annotations;
  • text-align;
  • word-spacing.

Readium CSS currently expects this flag for their associated --USER__property to take effect, and will not apply then if it’s not set. But in theory, ReadiumCSS should not care at all, and remove it from the selectors to enfoce them no matter what.

I’m considering this flag doesn’t have any meaningful use any longer in Readium CSS itself, but obviously a Publisher’s styles toggle may be available in apps as it’s often used as a shortcut to setting/unsetting all or a subset of the settings listed above.

The options that have been discussed semi-publicly so far are:

  • Should it be kept in ReadiumCSS even if it’s not meaningful, as it helps implement such a toggle as a shortcut w/o having to deal with setting/unsetting?
  • Should it be removed so that implementers can manage what they want to do with such a toggle entirely?

With early feedback in favour of removing it entirely.

Unless there are good reasons to keep it, it is likely this flag will be removed, which is why I’m opening this issue so that there’s time for developers to provide feedback and opinions.

@JayPanoz JayPanoz added the 2024 Update New discussions, issues and requests triggered by the 2024 update label Mar 18, 2025
@JayPanoz JayPanoz self-assigned this Mar 18, 2025
@danielweck
Copy link
Member

isn't there a font-size fallback for when zoom is not available? (in which case the advanced flag would still be necessary?)

@JayPanoz
Copy link
Collaborator Author

JayPanoz commented Mar 18, 2025

@danielweck that's correct, although the nuance is that the new fontSize submodule is applying the font-size without it being present in the selector for both zoom and its fallback.

Only if you want to normalize you have to set the flag for submodule fs-normalize's styles to take effect, the font-size on :root itself is no longer tied to it.

This means the fontSize setting could well work even if zoom is not supported and the fallback kicks in (i.e. book styles values are using relative units), and the flag could then be used to force the normalization onto problematic publications, either through a setting or some programmatic check. Or you could set it by default.

I forgot to list the option of updating the meaning of this flag and this use case, apologies.

But for other settings that would allow developers to cherry-pick what they want to include in a publisher's styles toggle, and make ReadiumCSS less opinionated.

@JayPanoz
Copy link
Collaborator Author

Update: having built Playground’s settings affordances w/o publisherStyles in mind, I am now strongly in favour of removing it from user settings submodules and recycling/renaming it for the sole purpose of font-size normalisation in case zoom does not work and the font-size user setting doesn’t have any effect cos the publication styles are declared in absolute units.

It makes things a lot more flexible, allows to pick what you want to put under a publisher styles toggle, and there is no side-effect I can think of – adding the CSS Prop makes it work, removing it resets to publisher’s style.

Actually, I’m currently adding the advanced flag by default for all advanced settings and never removing it, I’m only nullifying the preference and Navigator removes what needs to be removed through the Preferences API in ts-toolkit. This should simplify the PreferencesEditor as well. In practice, it’s also accidentally applying the font-size normalization for browsers that don’t support zoom since the normalization module is guarded in ReadiumCSS.

Does that sound good to everyone or are you anticipating unexpected side effects? Pinging @danielweck & @mickael-menu who have been the 2 major consumers in production.

@danielweck
Copy link
Member

sounds good to me.

@mickael-menu
Copy link
Member

That's great, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2024 Update New discussions, issues and requests triggered by the 2024 update
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants