Problem/Motivation

Drupal currently uses common reset CSS rules to flatten default browser styles, sometimes removing useful styles. Only for these to be added back in later by themers and site builders.

Proposed resolution

Normalize.css a recent addition to core is an alternative to traditional CSS resets.

It supports a wide range of browsers (including mobile browsers) and includes CSS that normalizes HTML5 elements, typography, lists, embedded content, forms, and tables.

It retains many useful default browser styles.

You can read further about normalize.css here.

Therefore it is proposed to add normalize.css as the first loaded stylesheet on all Drupal installations, and remove conflicting styles from core modules and themes.

If this is not done, and core stylesheets such as system.base.css retain similar conflicting styles, then making use of normalize.css will produce many duplication's/overrides.

Remaining tasks

This needs discussion within the community to be sure we want to go this direction as stated by @sun in Clean up the CSS for System module

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ry5n’s picture

I support using Normalize in all core themes that provide CSS at all. However, Normalize should not be added by default to just any theme. I have used various tweaked versions of Normalize in production, and keep in mind that Core's version is 2.0.1. Some sites will want to use something else, including Normalize 1.x for legacy browser support. Adding Normalize to all themes would create yet another case of Drupal making a (well-meaning) assumption that doesn't hold, forcing folks to figure out how to remove stuff they don't want.

I do support the idea of making it more visible to themers, so they know they can use it and how. Partly that's good documentation, perhaps even a comment in every theme.info file, but I haven't really thought enough about that.

rolfmeijer’s picture

I can understand why someone would use normalize.css, but I don’t agree with the assumption that “all site developers and themers are having to fight against browser inconsistencies on every site”, do you have any factual numbers on that? I for one never used it.
Like @ry5n I don’t like the idea of enforcing this to all theme(r)s. The idea behind it may be to make things simpler, but instead it adds yet another level of complexity, more overrides in CSS, more stuff to learn on how this behaves.
Make it easy to add? Great idea! Make it standard? Sorry, please don’t.

idflood’s picture

I use normalize on all project but a sass version so that the reset already make the website like I want.

So i like the idea to add it to all core themes. This way we could clean up the css for system module. So I believe normalize should be handled like a library, just like other js things.

mbrett5062’s picture

From the comments so far, I believe my intentions here are not fully understood.

What I am proposing, along with adding normalize.css, is to remove from core modules and theme's all reset styles. This would reduce the complexity and overrides required, not introduce more.

Normalize.css only fixes older browser bugs, and major inconsistencies between browsers whilst adding styles for HTML5 block level elements. It does not do what a traditional reset.css is doing.

I quote from their site.

The aims of normalize.css are as follows:

  • Preserve useful browser defaults rather than erasing them.
  • Normalize styles for a wide range of HTML elements.
  • Correct bugs and common browser inconsistencies.
  • Improve usability with subtle improvements.
  • Explain the code using comments and detailed documentation.

Normalize.css preserves useful defaults

Resets impose a homogenous visual style by flattening the default styles for almost all elements. In contrast, normalize.css retains many useful default browser styles. This means that you don’t have to redeclare styles for all the common typographic elements.

Normalize.css corrects common bugs

It fixes common desktop and mobile browser bugs that are out of scope for resets. This includes display settings for HTML5 elements, correcting font-size for preformatted text, SVG overflow in IE9, and many form-related bugs across browsers and operating systems.

Normalize.css doesn’t clutter your debugging tools

A common irritation when using resets is the large inheritance chain that is displayed in browser CSS debugging tools.
This is not such an issue with normalize.css because of the targeted styles and the conservative use of multiple selectors in rulesets.

Also, if we only expose it for users to add on-top of current default styling, we will be increasing the noise/clutter, not decreasing and cleaning up.

I hope this clarifies some of the concerns.

ry5n’s picture

@mbrett5062 I don't think anyone is resisting this based on a misunderstanding of what Normalize is, or questioning whether it is better or not (it is better). We all seem to be open to the idea of replacing the crufty old resets in core with Normalize and therefore helping to clean up system css. I for one am hugely in favour of that. In fact, I was directly involved in replacing Seven's reset with Normalize, which is why the normalize 2.0.1 library is now in core :)

What I'm opposing is adding Normalize to any theme, core or contrib, no matter what. What I want to see is Normalize be opt-in for all themes, not opt-out.

sun’s picture

Title: Propose adding normalize.css first on all themes. » Add normalize.css to all themes

Over in #1217044: Clean up the CSS for System module, someone mentioned that some or most parts of our current system.base.css could potentially be replaced with normalize.css.

I'd like to learn whether that is actually the case, and if it is, I'd like to see concrete evidence. — Whereas concrete means a detailed comparison and analysis of system.base.css and normalize.css.

That's the only and most important aspect from my perspective. Everything else is secondary and borderline irrelevant, since we've added normalize.css as a proper library, so any theme can decide at any time as to whether it wants to use normalize.css or not. Doing so takes 3 lines of (extremely common) PHP code in total, of which 2 lines commonly exist already, effectively reducing it to a single line in most cases. Can hardly get easier.

@mbrett5062: Thanks for copypasting that sales and marketing material. A simple link to it would have been sufficient.

mbrett5062’s picture

@sun, A link was already present in the issue summary, I just did not at the time think that people had read it. I was wrong, they are aware of what it brings. Therefore I apologise for trying to hammer that home.

It is obvious now that no one needs convincing of the use of normalize to themers.

So instead, as you have requested, here is a reason for putting this in first before system.base.css and system.theme.css.

If we leave this as add in after, then system.base.css adds styles for HTML5 block level elements and system.theme.css adds image 'borders: 0' and 'borders-collapse: collapse' on tables.
And maybe a few others in core module CSS.

When a themer activates normalize he then gets duplicates as normalize does the same thing.

However if we remove the styles from core modules, and the themer does not activate normalize then his HTML5 elements will be rendered incorrectly. So he must activate it.

I am sure that Bartik does a lot of what normalize does including setting image style to 'border: 0' and tables to 'border-spacing: 0', so again duplicates. And probably all other theme's.

So here is the quandry, either leave duplication/clutter if you want normalize to do the extra things we currently do not do. Or remove the duplication, and load normalize first. Or add the bug fixes that normalize does to system module CSS, and leave normalize out all together.

I for one, would like to see the duplication removed, but also would like some of the extras that normalize brings. So either add it first, or incorporate into system CSS. I would also like system CSS cleaned up, removing a lot of the hard resets that then have to be wrangled with afterwards.

Anyway, I have said enough on this. I will leave it now for others to decide.

Good luck, and lets look forward to a cleaner, leaner, more modern Drupal.

sun’s picture

Status: Active » Postponed (maintainer needs more info)

I just had a look at the current system.base.css, and all that normalize.css would "duplicate" is this very small fragment at the end of system.base.theme:

/**
 * Block-level HTML5 display definition.
 *
 * Provides display values for browsers that don't recognize the new elements
 * and therefore display them inline by default.
 */
article,
aside,
details,
figcaption,
figure,
footer,
header,
hgroup,
menu,
nav,
section,
summary {
  display: block;
}

The entire rest of system.base.css has nothing to do with normalize.css.

Therefore, I stand by #6: Why do we need to force normalize.css on everyone, if its inclusion boils down to a 1-3 line(s) change in template.php?

All of the core themes can be changed to leverage normalize.css at any time.

JohnAlbin’s picture

Status: Postponed (maintainer needs more info) » Needs review
FileSize
3.26 KB

However, Normalize should not be added by default to just any theme. I have used various tweaked versions of Normalize in production, and keep in mind that Core's version is 2.0.1. Some sites will want to use something else, including Normalize 1.x for legacy browser support.

Not surprisingly, there is already a module to provide that functionality. http://drupal.org/project/normalize It has a toggle to switch between the 1.x branch and the 2.x branch of Normalize. Porting this D7 module to D8 will be trivial.

Why do we need to force normalize.css on everyone, if its inclusion boils down to a 1-3 line(s) change in template.php?

Because it isn't just a change in template.php. You also would need to override core's base styling (currently in system.base.css) so that you can remove the redundant rules in it that are now also in Normalize.css. Meaning you'd need to copy that file out of system module, put it in your theme, edit the file and then add it to your .info file.

I just had a look at the current system.base.css, and all that normalize.css would "duplicate" is this very small fragment at the end of system.base.theme

Actually, no. The full set of duplicate CSS is shown in this diff:

 /**
- * Block-level HTML5 display definition.
- *
- * Provides display values for browsers that don't recognize the new elements
- * and therefore display them inline by default.
- */
-article,
-aside,
-details,
-figcaption,
-figure,
-footer,
-header,
-hgroup,
-menu,
-nav,
-section,
-summary {
-  display: block;
-}
-
-/**
  * Form elements.
  */
 form {
@@ -32,9 +11,7 @@ form {
   padding: 0;
 }
 fieldset {
-  border: 1px solid #ccc;
   margin: 1em 0;
-  padding: 0.5em;
 }
 label {
   display: block;
@@ -45,7 +22,6 @@ label {
  * Table elements.
  */
 table {
-  border-collapse: collapse;
   width: 100%;
 }
 caption {
@@ -68,6 +44,3 @@ hr {
   border: 1px solid gray;
   height: 1px;
 }
-img {
-  border: 0;
-}

Alternatively, if we add Normalize.css by default, the “out-out process” is just: Add stylesheets-remove[] = normalize.css to your .info file. (As described in http://drupal.org/node/1876600 )

It also makes it easy for people who want to load a CSS reset before any other styles (instead of normalize.css). They can put their reset inside a file named normalize.css and then add this to their .info file:

; We've secretly replaced the fine normalize.css Drupal usually serves with a CSS reset. Let's see if anyone can tell the difference!
stylesheets-override[] = normalize.css

So to reiterate:

Adding Normalize.css is hard. Removing it is easy.

Here's a patch that will fail because it depends on #1924430: Add drupal.base library for base CSS styles

JohnAlbin’s picture

Title: Add normalize.css to all themes » Add normalize.css to all themes by default

Updating title to highlight that it is just a default. Removing it is easy.

Status: Needs review » Needs work

The last submitted patch, 1839318-9-add-normalize-by-default.patch, failed testing.

JohnAlbin’s picture

I just noticed I added a slight inconsistency in the #attach order between _drupal_maintenance_theme() and system_page_build(). Fixed that in this patch.

mbrett5062’s picture

Thanks for your support @JohnAlbin.

ry5n’s picture

I think my comment in #1 was wrong. This is quite easy to override now, and Normalize is a great base to build all our themes on. Consider me in support.

Here’s a related idea: even though the override is one line, beginners may not know it. Would it be possible (as a separate issue) to provide an ‘example theme’ in D8 that demonstrates common code like this in theme.info (and template.php)?

sun’s picture

In general, I'm in support for this.

It's a *huge* assumption though, which we generally try to avoid (or actually remove) in the theme space.

However, with http://drupal.org/node/1876600, removing that assumption could be considered piece of cake.

Though, perhaps I need to immediately disagree with myself, because:

Over time, adding normalize.css like this by default will have the consequence that every line of CSS in Drupal will actually depend on normalize.css.

Right now, that's not exactly the case. normalize.css is only loaded by Seven theme. Bartik mostly relies on system.base.css, plus some custom reset-alike rules in style.css. So while me might have hidden/obscure dependencies already, they are rather caused by coincidence.

Now, is that good or bad?

As mentioned, I'm generally in favor of normalize.css. It feels like the right way. But it's also a huge effort and massive undertaking in terms of a project, and I can only imagine how much time it takes for a single line to get cross-browser, cross-version, and battle-tested, until it is finally allowed to get into that file... Badly needed, and the fundamental approach looks much more sane than former reset.css approaches.

At the same time, all of us only know too well what it means when Drupal shits 4.7.x CSS styles from ~2005 all over us. Assumptions, dependencies, horror. We may focus on normalize.css today, but the reality in ~2015-2019 might look completely different. Thus, how many mistakes of the past are we going to repeat with this? Will it actually help Drupal to be and stay modern in the future, or will it actually cause the exact opposite & be a total drain for all Drupal themers on this planet?

Given how fast the front-end space is moving today, that's anything but trivial to predict. But IMHO, that's the essential question here. "Today" isn't really relevant - "one year from now" (at minimum), and +6 years ahead, is the time-frame that matters.

ry5n’s picture

@sun Seems to me Normalize is the best thing to depend on at the moment, and we’ll always be depending one some base CSS, either our own (as in D7) or someone else’s (e.g. Normalize). Everything always gets out of date, and some people are not going to want to use Normalize even now.

So it’s two things: first, keeping Drupal up to date, which is just something we need to do. Second, what should be opt-in and what opt-out? In general, I think most things should be opt-in. At BADCamp, there was this whole discussion about Drupal’s markup, and trying to strip it down to the real basics. Right now with D7 it’s so hard to get Drupal to output nice, clean HTML. However, with Normalize I think I come down on the side opt-out, since modules should be able to rely on something like it. For those developers who don’t want it the way to remove it should be easy and obvious. We have the easy part, it’s the obvious part I’m worried about. I’m tempted to propose that all theme.info files must include a comment block listing the assets (core and otherwise) that are loaded by default and how to remove each of them. The user could simply uncomment the relevant lines in their theme, or copy them to their sub-theme. Would that work?

RobLoach’s picture

Not every website requires a normalize.css... It should be up to the theme to determine whether or not normalize should be included. Yes, we can add it, and then override it to remove it, like sun mentioned, but at that point, it becomes opt-out rather than opt-in.

In many front-end frameworks (Bootstrap, SASS solution like idflood, etc), along with many Drupal base themes, something like normalize is already provided, and is already better tightly integrated than what is suggested here. In addition, with CSS3 around, along with modern browsers becoming the de-facto, the requirement of normalizing to a standard CSS base almost becomes mute. I'd suggest leaving this up to the theme to determine whether it's needed. Does Seven require this? Does Bartik require it? If not, then why include it?

sun’s picture

@Rob Loach: Seven uses normalize.css already.

The main point that is being made here is that system.base.css contains some similar base styles as in normalize.css already. Thus, when loading normalize.css for all themes, we could send some of those duplicate rules in system.base.css to /dev/null

In general, we rather prefer the opt-in approach, and thus we should definitely add support for loading libraries to theme .info files; i.e., like this:

libraries[system][] = normalize

Or in case #1793074: Convert .info files to YAML will happen, it would look like one of these:

libraries:
  - [system, normalize]
---
libraries:
  system: [normalize]

The counter-argument to be made is that pretty much everyone would copy those lines into pretty much every theme .info file.

And the additional argument on top of that is that everyone who doesn't want it can always easily do:

stylesheets-remove[] = normalize.css

So these are some good points for going with an opt-out approach, too.

Shyamala’s picture

Issue tags: +mobile, +d8mux, +d8mux-css-cleanup

Adding tags

carwin’s picture

#18 sums this up succinctly.

Honestly, we may as well flip a coin. Whether we opt-in or opt-out, some people are going to have to add a line to their .info file.

alanburke’s picture

I'd consider normaize.css part of front-end best practice right now.
Strong support here - especially as we have the option to remove or override quite easily.

sun’s picture

I'd recommend to move forward with normalize.css as default library for all themes here.

That said, this is under the assumption that I hope this means that #1924430: Add drupal.base library for base CSS styles inherently won't fix with that.

mbrett5062’s picture

Further to the discussion here, the following is how I think we should proceed. I realize it is a large undertaking, but feel strongly that it is something we should strive for.

Stark - use only normalize.css to show what Drupal looks like across all browsers, with minimal to no styling.
Seven - use normalize and only add those styles required for admin pages, and overlays. No other theme styling.
Bartik - start with normalize and show what can be done to create a basic Drupal theme, including what we have currently in our modules, without the additional resets already in normalize.
All modules - remove css entirely.

We then need to document this approach, clearly, including how to override/remove normalize.css.
Normalize will need to be updated regularly, as and when required. (We could also help support it, as we do with Symfony and CKEditor.)

What this gives everyone who themes Drupal is a clean slate, and 3 different approaches to themeing.
1) Remove normalize.css and start entirely from a clean slate.
2) Start from Stark, and style everything yourself with a good cross browser grounding.
3) Start from Bartik, and restyle to your own liking, but keeping many of the default styles for module content, i.e links, pagers, menus and such.

I think this approach will give the best of all worlds to everyone, and will prove easier to maintain as the landscape changes.

It will also help theme designers, and those that wish to just jump in and hack their way to a their own theme. It should be easier to document and help to educate new users. No more searching across all the different module directories, and no more need to keep overriding module css.

joelpittet’s picture

@mbrett5062 That is a great plan of attack IMO!

Could you elaborate on this a bit?

All modules - remove css entirely.

This sounds like introducing a bunch of CSS that will never be used from modules that are either turned off or never turned on. All the rest is crystal clear to me:)

mbrett5062’s picture

In more detail, here is what I see going forward.

Stark - As stated above should only load normalize.css

Seven - Loads normalize.css + seven_layout.css + component.css + states.css + seven_theme.css

Bartik - Loads normalize.css + bartik_layout.css + component.css + states.css + bartik_theme.css

Note that layout and theme are the only css files that change per theme.

For contributed theme's, the developer can then choose to start from scratch, or load Normalize, Component and States, then produce there own layout and theme styles, or any combination therein.

Module developers should be encouraged to make use of normalize, component and states. And indeed all core modules, must use these, and only theme their admin pages if required or use seven_theme if appropriate.

I hope that helps, it should give us an easier to use and maintain CSS base. All components being styled the same throughout core, i.e. pagers, blocks, form components, and such. Obviously layouts and theme's (colors, fonts and such) are unique to each theme.

I get that some CSS loaded will be redundant, but as the styling for components should be kept to the minimum required to present the component this will not be too much of a problem. I.E if Bartik, or a module does not use pagers for instance, then that snippet will be redundant, but I see this as a minor inconvenience relative to the huge gain in maintainability.

ry5n’s picture

Although I agree with the spirit of #23 (and have comcerns about details of #25), they’re a bit beyond the scope of this issue. We should focus on normalize for all core themes yes/no and patches. Broader discussion should go in other issues, say #1921610: [Meta] Architect our CSS and #1854344: [meta] Goals for core themes: Make Stark as semantic as possible; Make Bartik a better prospective base theme.

Snugug’s picture

I am not a fan of including Normalize in Core. Core should be providing as little CSS as possible, not including full CSS libraries. The level of support for Normalize extends beyond just 1.x or 2.x, but into what browsers you're looking to support as well. Take a look at the Normalize Compass extension; it will produce different CSS depending on your level of IE and Mozilla support. This is a per-project decision.

I'd much prefer Core to come with neither a reset nor a normalize (really, I'd prefer Core to come with NO CSS). For this reason, I'd prefer to not have Normalize in Core, and certainly not for ALL themes, although it can be explored for some Core themes.

Really, y'all are killing me with forcing me to remove thing from Drupal just to be able to create a non-broken theme. Stop it. Please.

rudiedirkx’s picture

I'm fine with this as long you can remove normalize.css from the CSS list with the usual css_alter etc. Normalizing, resetten etc should be up to the developer, not Drupal itself. Sort of like choosing which WYSIWYG to use.

James Lefrère’s picture

I have to take issue with the problem this is supposed to fix:

Drupal currently uses common reset CSS rules to flatten default browser styles, sometimes removing useful styles. Only for these to be added back in later by themers and site builders.

Talk to most themers and you will hear a different story. We spend so much time and effort removing or overriding core/contrib styles, it's sometimes ridiculous the effort that themers have to go to in order to get unwanted CSS out of the way. See these examples:

https://drupal.org/project/stylestripper - Removes core CSS
https://drupal.org/project/mothership - Removes core CSS, cleans markup
https://drupal.org/project/aurora - Removes core CSS

It's a quick win to simply not include normalise for non-core themes and leave it to the sitebuilder; forcing it would be (in my opinion) bad DX (developer experience) for us frontend folk.

LewisNyman’s picture

This is inconsistent with the direction of D8. We have two initiatives that focus on cutting down on core's default output, adding another default is the opposite approach.

#1980004: [meta] Creating Dream Markup
#1921610: [Meta] Architect our CSS

JohnAlbin’s picture

Status: Needs work » Needs review
FileSize
2.71 KB

#1924430: Add drupal.base library for base CSS styles landed and it has put all the "base HTML element" styling into one file.

Really, y'all are killing me with forcing me to remove thing from Drupal just to be able to create a non-broken theme. Stop it. Please.

I understand the frustration. There's tons of broken CSS in Drupal core. That's why we are trying to clean it up. (See #1921610: [Meta] Architect our CSS) However, prove to me that normalize.css is in any way "broken".

Nicolas Gallagher is trying to deprecate normalize.css by making all browsers use the same styling for HTML elements. +1 Until then, all of our well-defined components are built on top of an inconsistent base due to browsers inconsistencies. That's the point of normalize.css in the interim.

By adding normalize.css by default, we can also remove a bunch of rulesets in the drupal.base.css stylesheet, which is already added by default.

Status: Needs review » Needs work

The last submitted patch, 1839318-31-normalize-all-the-things.patch, failed testing.

Snugug’s picture

There is nothing wrong with normalize per se, it's the notion that Drupal should ship with it by default and all themes are, by default, forced to use it. THAT is the issue. It's even more CSS in Drupal Core that has no business being there. Additionally, I'm actually not suggesting that Normalize is broken, I'm suggesting that I need to build broken themes. The perfect visualization of this is to install Magic module, have Seven as your admin theme, and remove Core's CSS while keeping Seven's

:core
~:current-theme

That is a broken theme. That is something that is in charge of look and feel that is broken, out and out broken, without the crap that Core comes with. That's what I mean by wanting to build non-broken themes.

What's worse about including Normalize is that there are other solutions out there that are equally valid (the biggest one being the Eric Meyer CSS Reset). By including Normalize, Core is making a design decision for the front-end. Additionally, the very first thing themes should be doing is creating a base set of styling for all of their elements, usually referred to as a style guide, that exists to deprecate the need for Normalize as it's purpose is to exist as a site-specific Normalize.

JohnAlbin’s picture

Additionally, the very first thing themes should be doing is creating a base set of styling for all of their elements, usually referred to as a style guide, that exists to deprecate the need for Normalize as it's purpose is to exist as a site-specific Normalize.

Yes, exactly. I'd expect any well-written theme will override normalize.css with their own styling. In fact, I always take Normalize.css as my starting point and then update that file with the default styling I want for HTML elements on that particular site.

By providing normalize.css as a default, we're encouraging good behavior. Namely: make sure you provide styling for all the HTML elements in your design. (BTW, that's my major complaint with reset styles. It removes all styling and 99.9% implementors fail to put styling back on all HTML elements. And resets are not future-friendly since they break all new HTML elements.)

JohnAlbin’s picture

Actually after applying the patch in #31, the drupal.base.css file just becomes:

/**
 * Form elements.
 */
form {
  margin: 0;
  padding: 0;
}
fieldset {
  margin: 1em 0;
}
label {
  display: block;
  font-weight: bold;
}
input {
  /* Keep form elements from overflowing their containers. */
  max-width: 100%;
  -moz-box-sizing: border-box;
  -webkit-box-sizing: border-box;
  box-sizing: border-box;
}

/**
 * Table elements.
 */
table {
  width: 100%;
}
caption {
  text-align: left; /* LTR */
}
[dir="rtl"] caption {
  text-align: right;
}
th {
  padding-right: 1em; /* LTR */
  text-align: left; /* LTR */
}
[dir="rtl"] th {
  text-align: right;
  padding-left: 1em;
  padding-right: 0;
}
thead > tr {
  border-bottom: 1px solid #000;
}
tr {
  border-bottom: 1px solid #ccc;
  padding: 0.1em 0.6em;
}

So…

  1. Guh. All those table rules are horrible defaults. They are okay-ish for tables inside forms, but we already decided long ago that the Seven theme was going to use some other styling. Those table rules were around in Drupal 4.6 and possibly before. Let's all agree to just remove them, okay?
  2. Now for the form element rules… the <input> is especially important since it prevents form breakage on mobile. What if we just move those to Seven and Bartik?

If we do both of those 2 points, then we could remove the drupal.base.css library and just turn on normalize.css for all themes.

So this issue would become: Replace drupal.base.css library with normalize.css for all themes

Does that work for everyone? I'll roll a new patch.

Snugug’s picture

I am more OK with including normalize if it totally replaces drupal.base.css

JohnAlbin’s picture

Title: Add normalize.css to all themes by default » Replace drupal.base.css library with normalize.css for all themes
Status: Needs work » Needs review
FileSize
6.3 KB

I've moved what was left of drupal.base.css into various appropriate rules in Seven’s and Bartik’s style.css files.

And I've nixed the library declaration of drupal.base and the CSS file from misc.

And replaced every instance of drupal.base being added to the system with normalize.

Status: Needs review » Needs work
Issue tags: -mobile, -d8mux, -Front end, -d8mux-css-cleanup

The last submitted patch, 1839318-37-replace-drupal-base-css.patch, failed testing.

JohnAlbin’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +mobile, +d8mux, +Front end, +d8mux-css-cleanup

The last submitted patch, 1839318-37-replace-drupal-base-css.patch, failed testing.

JohnAlbin’s picture

Status: Needs work » Needs review
FileSize
8.48 KB

So, before this patch, there was a bug such that Seven's install page wasn't loading normalize.css. So when I added it for all themes, the password checker styling in user.module needed tweaking so it didn't mis-align on the install screen.

Also, figured out why the install was failing for testbot and fixed that.

echoz’s picture

Oops, took too long to review and there was a change in system.module that made #41 not apply, so here's the updated patch.

Unrelated, I do think it's a shame that all themes don't get the "Keep form elements from overflowing their containers" fix.

sun’s picture

I do think it's a shame that all themes don't get the "Keep form elements from overflowing their containers" fix.

Same also goes for the form and fieldset styles (unless they are guaranteed to be obsolete today).

In general, I do not think it is wise to force everyone to re-invent the wheel.

Convincing upstream would certainly be best, but appears to be unlikely for these cases.

RainbowArray’s picture

Assigned: Unassigned » RainbowArray

I'll try some manual testing on this later today.

RainbowArray’s picture

I tested out the patch. Confirmed that drupal.base.css is removed both for Bartik and for Seven and normalize.css is also removed for both.

I also confirmed that the form rules were moved to both Bartik and Seven: however, the input rules were not moved into Seven. Is there a reason they aren't included there?

Other than that, this looks good.

echoz’s picture

Seven has the same rules in a slightly different way, for individual input classes.

Did you mean to say normalize.css is removed for these core themes? They are supposed to get it. As an aside, Seven has been using normalize.css since it was added as a core lib.

RainbowArray’s picture

Sorry I got that the wrong way around. drupal.base.css is removed, normalize.css is added. As it's supposed to do so. So, is this ready to go RTBC?

RainbowArray’s picture

Status: Needs review » Reviewed & tested by the community

Looked at one more time with John. This looks good to go!

RobLoach’s picture

webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Patch no longer applies.

echoz’s picture

Assigned: RainbowArray » Unassigned
Status: Needs work » Needs review
FileSize
8.49 KB

again...

JohnAlbin’s picture

Status: Needs review » Reviewed & tested by the community

The interdiff between my patch in #41 and #51 is:

< -    'version' => VERSION,
---
> -    'version' => \Drupal::VERSION,

So back to RTBC.

tstoeckler’s picture

Status: Reviewed & tested by the community » Needs review

I don't think the problems highlighted in #42 / #43 have been adressed sufficiently. The form styles discussed here totally make sense in something called 'drupal.base', IMHO.

echoz’s picture

The removal of (recently added) drupal.base.css was a compromise to feedback that did not want extra css in core. @tstoeckler, do you have a counterproposal?

ry5n’s picture

I’d rather see themes take over as much responsibility for CSS as possible. My feeling is that having normalize loaded by default is plenty. There is always going to be some common styling used by many sites (like box-sizing: border-box, max-width: 100% for fluid images), and yes, the form max-width fix too. But we shouldn’t try to do dev’s jobs for them – this is what D7 tries to some extent, and it’s frustrating for those with experience. Starting with less on core is a good thing. IMO “starting with more” is where starter themes and contrib modules come in.

tstoeckler’s picture

extra css in core

Well it seems that CSS is not so much extra if ~99% of themes have to add those same lines back to get i.e. functional text inputs on mobile. I totally agree with the approach of reducing core's CSS footprint but I think this is going a tad too far.

That said, I'm not really an expert on the newest CSS efforts so I might totally be wrong, but the patch that currently removes general CSS in something called "drupal.base" and adds it in two separate themes just feels wrong to me.

Jeff Burnz’s picture

#42/#43 yeah I think those styles are now redundant with normalize replacing fieldset reset, although it does add legend reset which afaict we never really had before. labels as bold is a design decision that can be retired imo.

So really I am not that convinced those form styles make sense in core with normalize taking over most duties.

The flexible inputs are a thorny argument and I can see pros and cons, but I think the decision to remove them is prudent. If you understand the technique and that its half of the flexible input equation then its OK, otherwise its flawed and will cause minor issues. Also as ry5n points out its making a design decision on behalf of all themers and we should try to avoid that where possible. I'd have to say I'm on the fence with this one, its not that bad really but actually its safer and a better default to not have it, so yea, I think its prudent to remove it, erring on the side of caution so to speak.

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

Wow, that was a very precise and thoughtful response. Thanks very much for taking the time to explain this to a noobie in the new CSS world order!!!

It seems my/sun's/echoz's concerns are in fact invalid.

Edit: Add weirdly gone missing words. (twice)

JohnAlbin’s picture

Thanks, Jeff, for the explanation. And thanks, Tobias, for the RTBC. :)

alexpott’s picture

Title: Replace drupal.base.css library with normalize.css for all themes » Change notice: Replace drupal.base.css library with normalize.css for all themes
Priority: Normal » Major
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

Committed 4b72c96 and pushed to 8.x. Thanks!

tim.plunkett’s picture

#2100571: Do not add normalize.css to stark is a follow-up, and the fact that normalize.css is loaded for all themes should be documented as part of this change notice.

joelpittet’s picture

Issue tags: -Needs reroll

Thanks this should help some people and we still as usual can remove/replace it if necessary in our own themes. Also totally agree with stark having no help what so ever, bare minimum thanks @tim.plunkett. When developers want to start from scratch then that would be a great starting point.

FYI, I use normalize on almost every project but will probably grab the upstream one or a sass library to replace the one in core.

My thoughts is that if it helps the admin themes in core and maybe some front-end developers starting out then it's a win:) Bartik is for those themers starting out, stark is for people wanting to start from scratch and contrib for the rest! :)

ursula’s picture

I am using a modified bartik-based theme, to which I added my own normalize.css.

After I pulled the newest version from git today, my theme's normalize.css was not recognized. Instead,http://example.com/core/assets/vendor/normalize-css/normalize.css was used.
After I renamed my own normalize.css file (and changed the name in the yml file), all was fine again.

Is this expected behavior? I didn't think from reading this thread that the core supplied normalize.css overwrites.

tim.plunkett’s picture

@ursula You should use the stylesheets-override property, see https://drupal.org/node/1876600

stylesheets-override:
  - normalize.css
tim.plunkett’s picture

Issue summary: View changes

Removed first line, inline with comments. mbrett5062

LewisNyman’s picture

Assigned: Unassigned » LewisNyman
Issue summary: View changes
LewisNyman’s picture

Status: Active » Needs review

My change notice proposal:

drupal.base.css replaced by normalise.css

drupal.base.css has been removed, normalise.css is being loaded for all themes in it's place.

LewisNyman’s picture

Assigned: LewisNyman » Unassigned
jwilson3’s picture

re #66: i think normalize.css is the american spelling, with a 'z' not an 's'. Should the change notice state how someone could remove or replace Drupal default normalize.css in both a theme, or via a module / alter method?

RainbowArray’s picture

Assigned: Unassigned » RainbowArray

Writing up a revised change notice.

RainbowArray’s picture

Proposed change notice:

drupal.base.css replaced by normalize.css

Summary
Previously, drupal.base.css contained numerous reset styles along with some default styles for tables and inputs. drupal.base.css was formerly named system.base.css.

The reset styles stripped default styling of HTML elements: for example setting all margins and padding to zero, as the defaults can vary between browsers. In contrast, normalize's approach is to set intelligent defaults that work consistently across browsers: for example providing the same margins and padding for elements, rather than zero margins and padding.

Read more about normalize.css.

Modified versions of the table and input styles in system.base.css have been moved to Seven and Bartik's style.css files: these styles are no longer loaded by default for every theme.

  • normalize.css is now loaded by default for all themes, rather than drupal.base.css: this changes the default styling for HTML elements and could affect the appearance of elements in themes and modules
  • If you want to include a different version of normalize.css, you can do so in your theme's info.yml file as follows:
    stylesheets-override:
      - normalize.css
  • To remove normalize.css entirely, you can do so as follows:
    stylesheets-remove:
      - normalize.css
RainbowArray’s picture

Assigned: RainbowArray » Unassigned
RainbowArray’s picture

Title: Change notice: Replace drupal.base.css library with normalize.css for all themes » Replace drupal.base.css library with normalize.css for all themes
Status: Needs review » Fixed
Issue tags: -Needs change record

I added the change notice: https://drupal.org/node/2168417

RainbowArray’s picture

Priority: Major » Normal

Status: Fixed » Closed (fixed)

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