-
Notifications
You must be signed in to change notification settings - Fork 22
Decorator consistency #173
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
base: develop
Are you sure you want to change the base?
Conversation
This is more consistent with the ids and classNames we use across the ts-toolkit packages.
Consistency with existing Readium-injected features.
chocolatkey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding data-readium is a good idea.
If you've tested removing mix-blend-mode and it worked, then I'm fine with this change. I essentially brute-forced this solution while at DM and testing with light and dark mode styling at the time.
|
Glad you said that because I can see issues on testing again. Looks like I forgot to replace I will put that back in draft, as it's quite a nasty one – we can no longer derive dark mode from I'll take time to check how Thorium Desktop got around this issue as I can recall Daniel telling he had to do that too. |
This reverts commit 820699f.
Adjust sRGB, remove helpers that were not tested or do not handle features properly, check dark/light using actual contrast against black and white
In case we do not find a backgroundColor or we cannot convert it to rgba (e.g. color names, unsupported format…)
|
So yeah it is really hard to do without I've improved the color helpers slightly, and removed the one that could not be tested properly. There are two obvious issues that can be addressed immediately:
RGB and HEX are covered – adding the computed style should return rgba directly so there's that –, but if we wanted to cover more, I guess we would need a dependency such as this one. |
This builds upon the discussion in #170
Removesmix-blend-modefrom fallback implementation when the highlight API cannot be used so that there is no side-effectr2-highlight-0toreadium-highlight, and addsdata-readium=trueto the templated fallback, to follow the current pattern when injecting thingsIn practice I am not sure we need to add data-readium to the templated item since it already is in a container for which it is set, but at least it makes it clear it is injected by us if you query this item/className directly.