Problem/Motivation

The code in block.css has selectors that are too general. For example, it targets every list in every block and forces padding on them even if the component is meant to look individual to everything on the page.

CSS should be component specific and block is a too general term, especially when the selector is combined with html elements.

It is causing us to either overwrite the styles all over the place.

or

It is causing visual regressions in RTL where we need to force styles to override the block.css code in RTL for specific components, that's not how RTL rules are supposed to be. See... #2502135: Bartik footer block menu-item misaligned in RTL.

or

There are also very obvious visual bugs in Bartik right now caused by the code in block.css....

Proposed resolution

Find where these list styles are needed to be used and add them to that specific component only.

As far as I can see it's only for components that have printed bullet points and need the bullet points to not sit on the edge of the block. The example below is from the user login block....

With block.css styles

Without block.css styles.

Remaining tasks

Write a patch
Code review
Visually review as many blocks on the page to see if we have any regressions.
RTBC

User interface changes

None, apart from fixing any small visual bugs caused by the current block.css code, like the one I pointed out in the Problem/Motivation section.

API changes

None

Data model changes

None.

CommentFileSizeAuthor
#9 remove-block-css-from-bartik-2509902-9.patch2.83 KBemma.maria
#8 remove-block-css-from-bartik-2509902-8.patch2.32 KBemma.maria
#8 Screen Shot 2015-07-28 at 12.24.01.png64.57 KBemma.maria
#3 2509902-3.patch2.34 KBmunzirtaha
#2 2509902-2.patch2.31 KBmunzirtaha
Screen Shot 2015-06-22 at 12.04.15.png20.01 KBemma.maria
Screen Shot 2015-06-22 at 12.04.50.png19.44 KBemma.maria
Screen_Shot_2015-06-22_at_11_52_39.png60.53 KBemma.maria

Comments

emma.maria’s picture

emma.maria
she/her
commented
Title: The Block component code in Bartik targets too many components on the page. » The Block component code in Bartik forces CSS on too many other components on the page.
munzirtaha’s picture

munzirtaha commented
Status: Active » Needs review
StatusFileSize
new 2509902-2.patch2.31 KB

@emma.maria: I am also in favor of removing many rules that needs to be overridden in too many places while not having a significant advantage. However, my starting patch would include core css files so I am not sure whether we can update the bug title and component or it's better to open a new bug for it. You decide. I will start with a simple clean up that would solve the issues you have mentioned beside deprecating my patches for issue #2502135 and issue #2501843.

munzirtaha’s picture

munzirtaha commented
StatusFileSize
new 2509902-3.patch2.34 KB

A new revision

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Related issues: +#2501843: item-list class inherited the wrong margin in RTL
manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented

@munzirtaha I think we have to do a cleanup of all RTL styles. which are not need just remove it as you did in #3. Is it making sense ?

@Emma Please share your views as well.

emma.maria’s picture

emma.maria
she/her
commented
Related issues: +#2509390: tags has a wrong right padding in RTL
emma.maria’s picture

emma.maria
she/her
commented
Title: The Block component code in Bartik forces CSS on too many other components on the page. » Remove block.css code and add the code to other components.
emma.maria’s picture

emma.maria
she/her
commented
Issue summary: View changes
StatusFileSize
new Screen Shot 2015-07-28 at 12.24.01.png64.57 KB
new remove-block-css-from-bartik-2509902-8.patch2.32 KB

@munzirtaha so the idea I had for this was to remove the offending block.css code that is causing so many visual problems all over the place. And put that code in within other component files where it was specifically needed.

We also can't affect too much in Core code also, this is just a problem with Bartik's code specifically.

I have created the following fresh patch to show what I mean.

Also screenshot.

emma.maria’s picture

emma.maria
she/her
commented
StatusFileSize
new remove-block-css-from-bartik-2509902-9.patch2.83 KB

Oops a file didn't make it into the patch. Here's a better one!

lewisnyman’s picture

lewisnyman
he/him
commented 
+++ b/core/themes/bartik/css/components/menu.css
@@ -0,0 +1,13 @@
+/* The ul is required to win over specifity of ul.menu in system.theme.css */
+ul.menu {

It would be great to fix the code in system.theme.css so we don't have to add ul to this. Once #2395853: Split system.module.css and system.theme.css files into SMACSS style components is committed, we can work on an issue to fix the CSS in system/menu.theme.css

I'm happy to RTBC this as is, but only if we have the chain of issue dependancies in place.

It would look like this:
#2395853: Split system.module.css and system.theme.css files into SMACSS style components -> Fix system/menu.theme.css -> Fix Bartik/menu.css

emma.maria’s picture

emma.maria
she/her
commented
Title: Remove block.css code and add the code to other components. » Remove block.css list styles and add the code to other components.
lewisnyman’s picture

lewisnyman
he/him
commented
Related issues: +#2550805: Clean up menu.theme.css inline with our CSS standards

I've created the issue that prevents this file from being completely fixed. #2550805: Clean up menu.theme.css inline with our CSS standards

emma.maria’s picture

emma.maria
she/her
commented
Status: Needs review » Closed (duplicate)

Block list CSS was removed as part of this issue #2548805: Add sensible base layout styles for lists in Bartik. so now there is no code to remove. Setting this to Closed (duplicate).