-
Notifications
You must be signed in to change notification settings - Fork 22
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
Comments
isn't there a font-size fallback for when zoom is not available? (in which case the advanced flag would still be necessary?) |
@danielweck that's correct, although the nuance is that the new 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 This means the 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. |
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 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 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. |
sounds good to me. |
That's great, thank you. |
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 rewrittenfontSize
submodule (usingzoom
), we don’t have to normalize anymore.Now lots of settings depend on this flag:
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:
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.
The text was updated successfully, but these errors were encountered: