Remove lists.css from Bartik, add it's current code directly to the components it references. [#2408653]

Follow-up to #1342054: [META] Clean up templates and CSS

Problem/Motivation

Bartik's code needs to meet current coding standards in Drupal.
Lists.css currently only contains component specific list styles. As part of the coding standards, styles for components need to be kept together in one file and not be split across files.

Proposed resolution

We need to split out all of the styles in lists.css into the correct components and remove the file from Bartik.

Remaining tasks

Write a patch.
Visual review of the patch + screenshots - to check we do not visually change/break anything in Bartik.
Code review of the patch - to check the standards have been met correctly.

User interface changes

None, we are cleaning up files and code.

API changes

n/a

Comment	File	Size	Author
#15	bartik-remove-listcss-2408655-15.patch	1.34 KB	emma.maria
#13	Screenshot 2015-01-27 21.16.31.jpg	302.36 KB	lewisnyman
#11	bartik-remove-listcss-2408655-11.patch	418 bytes	emma.maria
#7	2408655 - BackstopJS Report.pdf	1.49 MB	lewisnyman
#5	interdiff-2408655-1-5.txt	522 bytes	DickJohnson
#5	bartik-remove-listcss-2408655-5.patch	4.24 KB	DickJohnson
#1	bartik-remove-listcss-2408655-1.patch	4.24 KB	DickJohnson

Comments

Comment #1

DickJohnson commented 16 January 2015 at 15:57

Status:

Active

» Needs review

Status	File	Size
new	bartik-remove-listcss-2408655-1.patch	4.24 KB

Pager and tips related things are duplicates at the moment so removing those.

For block related styles I created new file.

Content related stuff went to content.css.

Item-list related stuff got a new file called item-list.css.

ul.menu li { margin: 0 } didn't do anything so I removed it.

Sending to testbot.

Comment #2

16 January 2015 at 16:18

Status:

Needs review

» Needs work

The last submitted patch, 1: bartik-remove-listcss-2408655-1.patch, failed testing.

Comment #3

16 January 2015 at 19:25

Status:

Needs work

» Needs review

DickJohnson queued 1: bartik-remove-listcss-2408655-1.patch for re-testing.

Comment #4

16 January 2015 at 19:47

Status:

Needs review

» Needs work

The last submitted patch, 1: bartik-remove-listcss-2408655-1.patch, failed testing.

Comment #5

DickJohnson commented 17 January 2015 at 10:24

Status	File	Size
new	bartik-remove-listcss-2408655-5.patch	4.24 KB
new	interdiff-2408655-1-5.txt	522 bytes

Ok, so there was a typo.

Comment #6

DickJohnson commented 17 January 2015 at 10:25

Status:

Needs work

» Needs review

Comment #7

lewisnyman

he/him

commented 23 January 2015 at 14:21

Status:

Needs review

» Reviewed & tested by the community

Status	File	Size
new	2408655 - BackstopJS Report.pdf	1.49 MB

I diffed the pages using BackstopJS and it's all clear! I've attached the report.

Comment #8

alexpott

he/they

English

🇪🇺🌍

commented 25 January 2015 at 13:04

Status:

Reviewed & tested by the community

» Fixed

+++ /dev/null
@@ -1,67 +0,0 @@
-ul.menu li {
-  margin: 0;
-}

This seems to the be only css rule that is being removed. Are we sure that this is ok? Seems to be as this is a duplicate of the rule in system.theme.css

ul.menu li {
  padding-top: 0.2em;
  margin: 0;
}

CSS changes are permitted during beta. Committed 0f97b4f and pushed to 8.0.x. Thanks!

Comment #9

25 January 2015 at 13:05

alexpott committed 0f97b4f on 8.0.x

Issue #2408653 by DickJohnson, LewisNyman: Remove lists.css from Bartik...

Comment #10

emma.maria

she/her

commented 27 January 2015 at 20:03

Status:

Fixed

» Needs work

Just noticed that we have forgotten to remove list.css from the list of stylesheets in the info file to be used by ckeditor.

ckeditor_stylesheets:
  - css/base/elements.css
  - css/base/typography.css
  - css/components/captions.css
  - css/components/content.css
  - css/components/list.css
  - css/components/table.css

Core currently has a 404 error looking for the removed file.
We need to create a follow up patch to take this file out of the there asap.