Introduced by #639368: Contrast between error and link colour causes death to eyeballs ...

To reproduce:
- Install http://ftp.drupal.org/files/projects/update_test_module-7.x-1.0.tar.gz on your site (which you can do via the Update manager if you want!) ;)
- Either enable it at admin/modules or configure Update manager to check status on disabled modules at admin/reports/updates/settings
- Visit the available updates report at admin/reports/updates

You'll see something like this:

WTF is that? ;)

A) This whole box is a tr.error. It should be red.

B) The recommended version line's background is a lighter red since it's trying to highlight the line you care about, but without the box itself being red, it's just totally weird and misplaced as-is.

CommentFileSizeAuthor
#6 934148_available_update_colors_1.patch1.53 KBmarkabur
#6 available-updates-seven-after.png47.4 KBmarkabur
#6 available-updates-bartik-after.png47.32 KBmarkabur
#4 available-updates-seven-before-639368.png44.72 KBmarkabur
#4 available-updates-bartik.png42.84 KBmarkabur
#4 available-updates-garland.png50.07 KBmarkabur
#4 available-updates-stark.png52.11 KBmarkabur
639368-broke-available-security-release-colors-in-seven.png37.74 KBdww

Comments

dww’s picture

dww
we/he/they
commented

P.s. this is specific to seven. It looks "normal" in Bartik, Garland, et al...

markabur’s picture

markabur commented

Subscribe

markabur’s picture

markabur commented
StatusFileSize
new available-updates-stark.png52.11 KB
new available-updates-garland.png50.07 KB
new available-updates-bartik.png42.84 KB
new available-updates-seven-before-639368.png44.72 KB

Here are some screenshots for reference. Should Seven go back to the way it was before, or should it have a stripe like Stark and Garland do?

dww’s picture

dww
we/he/they
commented

IMHO, only Stark and Garland get this right. If everything is stripped like Bartik, it's not as obvious what you're supposed to install. If nothing is stripped like Seven, you have the same problem for the opposite reason. There's no "call to action". Granted, this whole page needs a lot of UX love (I guess that's going to have to wait to D8).

Anyway, the biggest bug here is that Seven doesn't use red for missing security updates on your site. That was probably by design in general for tr.error. But, I think the available updates page is a special case and we should restore the red. If the maintainers of Seven disagree, then we should at least remove the pink stripe.

I suppose the Bartik excess stripes should happen in a separate issue, huh?

markabur’s picture

markabur commented
Component: Seven theme » update.module
Status: Active » Needs review
StatusFileSize
new available-updates-bartik-after.png47.32 KB
new available-updates-seven-after.png47.4 KB
new 934148_available_update_colors_1.patch1.53 KB

The Bartik thing is sort of a separate issue but we're aiming for the same result so I guess we should just take care of it here.

Problem: tr.error background color incorrect in Seven. This was caused by a block of CSS that was added to Seven explicitly to override update.css, but it wasn't overriding anything useful that I could see, so I deleted it. Now Seven inherits the table.update styles from update.css just like Garland, Stark, and Bartik do.

-/* Available Updates */
-.update tr.ok {
-  color: #255b1e;
-  background: #e5ffe2;
-  border-bottom: 1px solid #89D47F;
-}
-.update tr.warning {
-  color: #840;
-  background: #fffce5;
-  border-bottom: 1px solid #ed5;
-}
-.update tr.warning .version-recommended {
-  background: #ffe;
-}
-.update tr.unknown {
-  background: #ddd;
-}
-.update tr.error {
-  color: #840;
-  background: #fffce5;
-  border-bottom: 1px solid #ed5;
-}
-.update tr.error .version-recommended {
-  background: #fdd;
-}
-table tbody tr.update-security,
-table tbody tr.update-unsupported {
-  color: #840;
-  background: #fffce5;
-  border-bottom: 1px solid #ed5;
-}

Problem: After removing the above CSS, Stark still applies its default table border to the left and right. This appeared to be unintentional, so I added the following higher-specificity rules to update.css:

+.update tr.even,
+.update tr.odd {
+  border: none;
+}

table.update,
 .update table.version {
   width: 100%;
   margin-top: .5em;
+  border: none;
 }

Problem: Bartik defines a gray background color for any tr in any table, which is showing up here on the inner table. Solved this by specifying "background: none" in a higher-specificity rule in update.css:

.update table.version tr,
.update table.version td {
  line-height: .9em;
  padding: 0;
  margin: 0;
  border: none;
+  background: none;
}

Now Bartik and Seven use the same colors and borders here as the other themes do, and there's less chance that other themes will inadvertently affect this table.

mgifford’s picture

mgifford
he/him
Primary language English
commented

subscribe

dww’s picture

dww
we/he/they
commented
Status: Needs review » Reviewed & tested by the community

I'm totally happy with #6.

- Removes unnecessary, ineffective (and visually broken) CSS from seven.

- Makes it so there's one place in core (update.css) to basically control the functional elements of this page, instead of having that sprinkled across the CSS from the different core themes.

I looked at my available updates report with all core themes as admin, and they're now consistent and functionally working as intended.

This is a CSS-only patch, and our test bot can't have tests for if the CSS "looks right", so this has been manually tested by a human (me) and it's great. No point waiting for the bot to tell us (which it inevitably will) that the core tests still pass.

RTBC!

Thanks,
-Derek

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.