Add drupal.base library for base CSS styles

Can we find a better file prefix than "drupal." ?

How about html.base.css?

Adding tags

Since this base CSS is now a core library, a Drupal `namespace` makes sense, doesn't it?

I re-rolled this against head. There were a couple of conflicts in there that I'm not too sure about, so this will definitely need some review.

Kim

#8: 1924430-8-drupal.base_.css_.patch queued for re-testing.

#8: 1924430-8-drupal.base_.css_.patch queued for re-testing.

CSS_BASE is not being defined anywhere. Just re-read the summary and can see it is part of #1924368: Rename old CSS constants to CSS_AGGREGATE_* and add new CSS weight constants

So much for a quick re-roll!

#8: 1924430-8-drupal.base_.css_.patch queued for re-testing.

Re-roll.

The #19 patch applies well and it seems nothing is broken in the css styling.

Also the drupal.base.css is loaded.

Is there something more I should take into account?

Fix status.

The CSS is all retained, it’s a straight consolidation, so that looks good.

I looked through system.module and found no straggling base styles except the new <details> and <summary> elements in system.theme.css. We can probably leave that for now, since <details> needs a js polyfill, has internal elements styled with classes, and may be better styled as a component anyway.

I can’t comment on the changes to theme.maintenance.inc or system.module, although everything seems to look correct with the patch applied.

#19: 1924430-19-drupal.base_.css_.patch queued for re-testing.

Hi,
rerolling #19
Thanks

The patch now applies well.

It seems right for me. Could anyone else confirm that?

If the two patches are compared, we see that #24 is not removing a whole section of system.theme.css that #19 is removing, which is still currently in the file (plus the button element is now present). Assuming #19 was correct at that time, this needs work.

Oh i missed that, Thanks
Here it is !

You left out drupal.base.css, what this issue is about. Don't forget there is the small ruleset for button, that was added since John's patch that has to go somewhere.

Hi,
taking into account the comments in #30.
Thanks

@izus, it's convenient to set our text editors to end files with a line break, as this is in our coding standards. #31 seems to have missed this, also mysteriously changed a comment, and failed to add that new button ruleset.

This patch should be an accurate rerole of the #19 patch.
I kept the new button ruleset with the button styles that it came with from #1719640: Use 'button' element instead of empty links in system.theme.css since I didn't think we want button {font-size: 1em;} in our drupal.base.css

#32: drupal-base-css-1924430-32.patch queued for re-testing.

Hi,
I reviewed #32 and it seems good for me.
Thanks

+++ b/core/misc/drupal.base.cssundefined
@@ -0,0 +1,83 @@
+article,
+aside,
+details,
+figcaption,
+figure,
+footer,
+header,
+hgroup,
+main,
+menu,
+nav,
+section,
+summary {
+  display: block;
+}
...
+  border-collapse: collapse;
...
+img {
+  border: 0;
+}

Most of these declarations are in normalize.css and therefore I think might be unnecessary.

From normalize.css...

article,
aside,
details,
figcaption,
figure,
footer,
header,
hgroup,
nav,
section,
summary {
    display: block;
}
...
img {
    border: 0;
}
...
table {
    border-collapse: collapse;
    border-spacing: 0;
}

... so we only need main and menu in the bit that ensures display values for browsers that don't recognize the new elements.

Good catch, although currently only Seven uses core's normalize.css. #1839318: Replace drupal.base.css library with normalize.css for all themes is not committed yet but seems like it will be, although that issue has been stalled since mid March.

#32 could go in with a followup to remove the redundant parts when #1839318 gets committed (Seven currently uses both normalize.css and system.base.css with the overlap), or this issue can get postponed until then.

Notes on the html5 elements:

The version of normalize.css in core is out of date.
"main" is now in both v1 and v2 of normalize.

menu is no longer in normalize, we can get rid of it.
https://github.com/necolas/normalize.css/issues/24

Related, hgroup is being removed from html5, that could go too.

I opened #1986616: Update normalize.css library with a patch to update core's normalize.css library.

If we want to go forward with committing this then following up with removing redundancies if and when #1839318: Replace drupal.base.css library with normalize.css for all themes gets committed, here is the patch with menu and hgroup removed.

Reference for hgroup removal, http://html5doctor.com/the-hgroup-element/ although this isn't gone from normalize.css.

Here's a re-roll of the patch. This looks pretty good to me? RTBC?

#40 leaves out drupal.base.css + drupal.base-rtl.css

Re-roll of #39

Re-roll, system.module has been getting touched often lately.

Yet another re-roll.

Checked. Patch applies well and drupal.base.css is loaded correctly. Go fo RTBC!

system.module touched again. Testing that it applies should be all that's needed to rtbc again. Let's get this committed!

I mistakenly re-rolled before learning that's not needed when just line #s changed. It's the same patch as #45.

This applies cleanly.

needs re-roll since #1987066: Rename files to match CSS file naming convention

re-rolled!

Patch applies well and drupal.base.css file loads correctly.

Also tried with a RTL language and drupal.base-rtl.css loads correctly too.

Go for RTBC!!

@JohnAlbin,
#37 explains the omission of menu and hgroup. Here's a resource link for hgroup too. http://html5doctor.com/the-hgroup-element/

#32 explains moving the button ruleset, but if you want that in drupal.base.css that's fine with me :-)

Here ya go...

Needs reroll

git ac https://drupal.org/files/drupal-base-css-1924430-57.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  7061  100  7061    0     0   3173      0  0:00:02  0:00:02 --:--:--  3511
error: patch failed: core/includes/theme.maintenance.inc:92
error: core/includes/theme.maintenance.inc: patch does not apply

This was a comment change, and now applies cleanly.

It applies perfect ;)

* Ops, I don't see the last update, sorry

#63: 1924430-63-base-html.patch queued for re-testing.

Finally, pass the test ;)

Needs a reroll

git ac https://drupal.org/files/1924430-63-base-html.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  6018  100  6018    0     0   4781      0  0:00:01  0:00:01 --:--:--  5769
error: patch failed: core/modules/system/css/system.module.css:337
error: core/modules/system/css/system.module.css: patch does not apply

+++ b/core/misc/drupal.base.cssundefined
@@ -0,0 +1,89 @@
+[dir=rtl] caption {
...
+[dir=rtl] th {

This should be [dir="rtl"] see #2030925: quote rtl attribute selector

reroll

the patch in #71 seems good !
Thanks

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

needed re-roll to quote rtl attribute selector since #2030925: quote rtl attribute selector landed.

Committed be502b4 and pushed to 8.x. Thanks!