Separate generic html elements' style into seven.base.css

I'm very much in favor of adding an html-elements.css file to style generic elements, it adds clarity and structure by seperating the typographic basics from the per-case specifics. It'll make a better impression on themers inspecting these files, too, it's just something you do :-)

It would be great to get this in soon, it gives us a better place to do other Seven cleanups.

Yes, good idea!

It might be something to adjust to this patch: #592018: Re-organize styles across stylesheets from system.module and separate presentational and behavior-supporting styles

By eventually just removing system.css from the stack to get rid of Drupal's default styles for HTML elements, and start with "naked" HTML in html-elements.css. (Or just override system.css, but introducing html-elements.css is probably better in this theme's context as it describes the stylesheet's purpose better.)

Openend about 10 admin pages before applying this patch. Refreshed all pages after applying. No visual changes whatsoever.
Looking at the newly created html-elements.css it looks like you picked all the basics from style.css that should be in here. Me wants!

errrr. "html-elements" ? Isn't all CSS about HTML elements...?

I find this filename quite confusing. Why not generic.css or general.css or base.css or something like that?

I am for this patch, this is the way I am theming by separating visual from the structure, like done here. Well done!

The only problem are the file name, like sun says. Any HTML elements can be styled as visual, structure or even behaviors through selectors.

Here are a few name suggestion for the new CSS file:

presentation.css
general.css
visual.css

From all suggestions, I'd prefer: base.css

presentation/visual is equally flawed, because all CSS is about that. ;) What this split tries to achieve is to move the generic/base styles away from more focused, special styles. That's a very good idea and also a very common practice. Let's just use a proper name for the file, so we can slowly set a standard here (which also means: doing the same for Garland wouldn't hurt).

OK. Let's go with base.css. :-)

Don't name this on personal preference please. It's not a vote. Zen theme is the 34th most used project on d.o. with 22,397 installations. (drupal.org/usage) Zen uses a html-elements.css for this. It would be smart to have core be informed by the most popular contrib theme for this so that we match expectations.

Whether it's coming from Zen or not does not make a difference. If you have a better suggestion than base.css (or generic.css), then let us know, please.

But html-elements.css is really a strange name.

#10:

What this split tries to achieve is to move the generic/base styles away from more focused, special styles.

#14:

technically all css does html elements but the distinction here is that styles in this file are for all occurrences of generic html elements

So we both naturally used the term "generic" to explain what this file contains. Let's go for it. ;)

Indeed! html-elements is not a good name. Let's use generic.css please. :-)

From http://www.w3.org/TR/html/#h-4.2:

XHTML documents must use lower case for all HTML element and attribute names. This difference is necessary because XML is case-sensitive e.g.

and

are different tags.

They are called html elements, so we should call this file html-elements.css. 'Generic' is meaningless, doesn't give *any* indication of what can be found inside.

You know, I think yoroy has a point here. When I work on drupal themes I always look for the style.css in the theme's folder. This is what a base.css or generic.css would mean to me. It's the root css file that has historically always been in play and the last css file I usually list in my .info file.

If we had another generically named file that has no other meaning in it's name other than it's lack of meaning then that would confuse me. I just keep thinking about how I would teach my non-drupal-enthusiast fellow programmers the purpose of a general.css and handling their needle-ly questions. Ironically, One solution I came up with is naming it something else. If the job that this specific css is responsible for is to modify the default presentation of standard html elements, then it does kind of make sense to call it html-elements.css or elements.css.

Subscribe, I don't have any really strong feelings towards the naming.
"base", "general", "generic" and to some extend "html-elements"(as in - there aren't any classes involved) make the most sense to me.
"presentation" makes the least sense to me as literally all css is for presentation.

I noticed that Zen changed the name from html-elements.css to html-reset.css. Might as well just call it reset.css, as in *duh* it's html. It makes sense to put all styles for generic html elements in reset.css; "smart resetting". Resetting everything to zero in reset.css and then styling everything differently in another stylesheet is not very efficient. What are your opinions on that?

@mverbaar: You seem to be saying that we should shoot a kind of css optimization that is achieved once there are no duplicative rules that apply on the exact same css selectors. I think that kind of optimization is admirable from a purist point of view, but largely useless once CSS aggregators/optimizers are used. Since Drupal 6, standard drupal installations have benefitted from using CSS aggregators to combine multiple css files in to one. It is possible to push the css further by running the combined CSS code through an CSS optimizer like CSSTidy. After seeing the benefit of those steps, I really don't worry about sub-optimimal or duplicative CSS code anymore.

So no, I don't see any harm in have a reset.css for the Meyer Reset, an elements.css for css applied to standard X/HTML elements, a style.css for developers to play with, and other theme css files. The benefit of the multi file approach is to provide developers with a clear segmentation of roles for these individual css files. You can basically tell a set of developers, "Only edit style.css, and if you absolutely need to edit any other css file, ask me first." (from a project manager's POV).

I'm not sure if you need to wait on #676800: Fieldsets break design badly but it probably won't hurt if you did. You may have to reroll the patch in #6 once it it's fully committed.

#6: separate-html-elements-2.patch queued for re-testing.

base.css would match our new CSS coding standards. I'd happy review a new patch with this naming convention.

@LewisNyman, isn't this covered (and obsoleted) in #1924430: Add drupal.base library for base CSS styles?
tldr, but marking this as duplicate of #1924430: Add drupal.base library for base CSS styles, correct me if I'm missing something.

@echoz I thought #1924430: Add drupal.base library for base CSS styles was just for the system module? This is an issue for the Seven theme. Did I misunderstand the other issue?

Ah, you're right. Seven's style.css has been certainly getting large also.

Had this patch lying around on my local for a while, I think we can get away with doing this now, the styleguide work is component-only right now.

Tested the patch and it seems to break styling.

This is the config page without applying patch:

This is the config page after applying #31 patch:

And this is the config page after applying this patch:

It seems to need more work but I want to confirm that I'm doing it well before continue.

Sorry, forgot patch and interdiff :(

For some reason the base.css wasn't being created in the previous patches, this one appears to apply fine. I've also renamed it to seven.base.css because I forget we had to do that.

There are still weird element styling, for example the installer and help pages. Should I fix it?

THIS should work. And this was supposed to be a novice issue...

Now works as expected. Great!! It's a RTBC for me.

Added screenshot to show that the issue is fixed ;)

Patch no longer applies, needs a reroll. Maybe during the sprint tomorrow in Prague? :)

Great idea @Cottser. Unassigned to let others re-roll it ;)

Rerolled

Ok, it seems to apply well. Let's wait for testbot result and RTBC ;)

#42: 641734-42.patch queued for re-testing.

Tags

Removing tags

Patch no longer applies.

Rerolled... but since we're stiring up seven's style.css, how about this being a good time to move seven's css into a css directory #2033211: Move Seven CSS files into css directory?

#48: 641734-48.patch queued for re-testing.

1. +++ b/core/themes/seven/style.css
   @@ -1,118 +1,12 @@
   -[dir="rtl"] .item-list ul {
   + }
   

   Looks like we added a space here before the closing bracket.

Apart from that I think we're good.

Space removed.

Thanks!

51: 641734-51.patch queued for re-testing.

51: 641734-51.patch queued for re-testing.

1. +++ b/core/themes/seven/style.css
   @@ -124,37 +18,16 @@ ul.menu li {
    .item-list ul li.collapsed,
    ul.menu li.collapsed {
   -  list-style-image:url(../../misc/menu-collapsed.png);
   -  list-style-type:disc;
   +  list-style-image: url(../../misc/menu-collapsed.png);
   +  list-style-type: disc;
    }
   

   I don't disagree with the changes here but to keep this patch simpler it might be better to move these changes to another issue.

2. +++ b/core/themes/seven/style.css
   @@ -124,37 +18,16 @@ ul.menu li {
    /**
     * Skip link.
     */
   @@ -456,6 +329,8 @@ ul.admin-list {
   
   @@ -456,6 +329,8 @@ ul.admin-list {
      display: block;
      padding: 14px 15px 14px 25px; /* LTR */
      min-height: 0;
   +  color: #0074bd;
   +  text-decoration: none;
    }
   

   This looks like something that creeped in. We shouldn't be adding styling here

@LewisNyman, this is the same patch you RTBC in #52, that for unknown reason got run through the test bot again in #54. On your #2 point, wasn’t that added for the cascade problem shown in #32 and #36?

@echoz Ah yes that's right, forget point 2.

@LewisNyman, so do you really want to add back in a coding standard error (no space after a colon) or are you ok with this back to rtbc?

+++ b/core/themes/seven/style.css
@@ -1,118 +1,12 @@
+[dir="rtl"] .item-list  ul {

If we're talking coding standards then as well as fix some this patch is adding some - unnecessary space added between .item-list and ul :D

+++ b/core/themes/seven/style.css
@@ -456,6 +329,8 @@ ul.admin-list {
+  color: #0074bd;
+  text-decoration: none;

Are you sure this is necessary? - I've applied this patch and removed these lines and /admin/config looks just fine to me?!

Seems like that css was needed at one time since that same code block had been used for the table th links, and are now picking that up from seven.base.css as well. I removed both instances of color: #0074bd; text-decoration: none; and have tested both with and without overlay, at admin/config, admin/help and admin/people (this last one for table header links).

63: 641734-63.patch queued for re-testing.

#63 patch had been saved with an extra ending line break.

67: 641734-67.patch queued for re-testing.

67: 641734-67.patch queued for re-testing.

Did not apply since #1986074: Buttons style update change in seven.info.yml

Ace, thank you!

73: 641734-73.patch queued for re-testing.

This should fix that one test fail.

I applied the patch, clicked around. Everything looks good.

77: 641734-77.patch queued for re-testing.

Committed ee9fe14 and pushed to 8.x. Thanks!

I'll be writing the change notice shortly.

Change Notice
Creates a new css file to separate base styles from theme-specific css.

Summary

• Moves generic css from style.css to a new file named seven.base.css
• Adds seven.base.css to theme.info.yml

Impacts
Themers

Change record created: https://drupal.org/node/2168399