MITLibraries / MITLibraries-child

A Wordpress child theme that descends from MITLibraries-parent
GNU General Public License v3.0
1 stars 1 forks source link

Explicitly sets child theme homepages to use white background #150

Closed matt-bernhardt closed 5 years ago

matt-bernhardt commented 5 years ago

Status

Use labels (review needed, in progress, or paused) to declare status

What does this PR do?

This adds some explicit styling to the child theme to set the home page to use a white background and black test. This became necessary because a recent update to the parent theme has allowed some sites to use a black-background styling, which wasn't intended. The markup that allows this to occur is inconsistently being applied, so it seems fastest to just explicitly set the colors here in the child theme for those sites to protect them from inconsistent behavior of the parent theme.

Helpful background context (if appropriate)

Ideally I'd like to identify the root cause of why the markup isn't being created correctly - but there are some high profile news items coming out today for the Scholarly site, so I'd rather just squash the problem this way and debug with more time later.

How can a reviewer manually see the effects of these changes?

Details with links are in Slack

What are the relevant tickets?

Screenshots (if appropriate)

Scholarly in production: image

Scholarly on the staging site: image

Todo:

Requires new or updated plugins, themes, or libraries?

NO

Requires change to deploy process?

NO

matt-bernhardt commented 5 years ago

I'm leaving the CodeClimate review alone for now, but will argue that the adjoining classes violation that is being flagged is needed. The rule being inherited from the parent theme is:

body.page-home {
    background-color: #000;
    color: #fff;
}

In order to trump this in the CSS cascade, I've elected to go with:

body.childTheme.page-home {
    background-color: #fff;
    color: #000;
}
matt-bernhardt commented 5 years ago

The violation in CSS Lint is inteded to point out that chaining styles is unsupported in IE 6 and earlier. Beyond the problem of supporting IE6 in 2019, the way IE fails (reading the end class only), would actually result in the intended outcome, because this rule is defined later in the the declaration list and thus would override the earlier style definition. I don't want to rely on this being later, though, so I'd prefer to leave this as a chained class.

https://github.com/CSSLint/csslint/wiki/Disallow-adjoining-classes