Skip to content
This repository has been archived by the owner on Sep 2, 2020. It is now read-only.

Fix appearance of spans with specific background color #211

Merged
merged 2 commits into from Jul 3, 2019

Conversation

joewalsh
Copy link
Contributor

@joewalsh joewalsh commented Jul 2, 2019

  • If a span has an inline style for the background, preserve the background color by applying the pagelib_theme_div_do_not_apply_baseline class
  • Set the color of pagelib_theme_div_do_not_apply_baseline to black since that's the assumed text color from the base css. Without this, the text could be set to a light color on dark themes on the background color that was intended for use with black text.

Before:
Simulator Screen Shot - iPhone X - 2019-06-26 at 08 49 45

After:
Simulator Screen Shot - iPhone X - 2019-06-26 at 08 49 17

@montehurd
Copy link
Collaborator

just a heads-up the pagelib theme demo seem to be broken - so i can't check if #211 causes any theming regressions on the ~50 or so hard-to-theme articles in theme demo. looks like it was working as of 6.5.0

@montehurd
Copy link
Collaborator

keep in mind the theme demo is slow to load since it's loading a bunch of pages - but once it loads and you pick a theme you can scroll through that single demo page and see all 50 of the tricky articles and whether they look correctly themed. makes it super easy to pick out unexpected regressions since you just pick black mode then scroll looking for unexpected bright areas.

@berndsi
Copy link
Contributor

berndsi commented Jul 3, 2019

just a heads-up the pagelib theme demo seem to be broken - so i can't check if #211 causes any theming regressions on the ~50 or so hard-to-theme articles in theme demo. looks like it was working as of 6.5.0

Good to know. I hadn't noticed since I usually use the AllTransforms demo these days.

@montehurd
Copy link
Collaborator

montehurd commented Jul 3, 2019

The theme transform loads now but the theme picker isn't appearing on top of the list of articles.

@berndsi Side note: I tried AllTransforms too but now get a JS error when trying to pick theme - is it working for you? Perhaps related to the refactor that happened a while ago? Btw I'm in favor of deleting the other demos if AllTransforms now suffices. If that's ok then the theme transform not working doesn't matter :)

@montehurd
Copy link
Collaborator

False alarm 🤦‍♂️

@berndsi berndsi merged commit 70010f5 into master Jul 3, 2019
@berndsi berndsi deleted the bug/color_box_template branch July 3, 2019 18:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
3 participants