Create generic layout classes

updated the patch and used a fluid layout system

ok, hope i redeem myself with this patch,

I guess we should use [dir="rtl"] statement for LTR selectors like margin, padding...

Work carried out in this patch.

1. Removed leftover margins and paddings on #page selectors in style.css that were clashing with layout.css.
2. The h1 title and breadcrumb on the overlay and pages were missing the new layout classes or not lined up with the rest of the page elements, so I've amended CSS and Twig files for them.
3. The overlay close wrapper that is positioned absolute was missing its positioning so I've added styles for that.
4. All CSS files that I have changed I have replaced the [dir=rtl] with [dir="rtl"] as per #2015789: Remove language_css_alter() (RTL stylesheets) in favor of HTML 'dir' attribute.
5. I have added "rtl" styles to layout.css
6. I also noticed that a max-width in layout.css was breaking the layouts (see pic) I have removed the max-width and all is well again.
7. There was also an IE7 fix within layout.css that shouldn't be there, so that has been removed too.

+++ b/core/modules/overlay/css/overlay-child.cssundefined
@@ -95,12 +97,12 @@ html[dir=rtl] {
+  margin: -28px 0 0 0; ¶

Remove trailing space :)

+++ b/core/themes/seven/layout.cssundefined
@@ -0,0 +1,68 @@
+    float:right;  ¶

Remove trailing spaces :P

+++ b/core/themes/seven/style.cssundefined
@@ -63,8 +63,8 @@ dl dl {
+[dir="rtl"] dl dl {
   margin-right: 20px;
 }

Should add margin-left: 0; here?

Thanks for pointing those things out. I have removed the trailing spaces and added the margin-left that was needed.

Patch applies well and code seems ok. Let's wait for "Green" and go fo RTBC!

+++ b/core/themes/seven/templates/page.html.twig
index a58d60e..fe16cfc 100644
--- a/sites/default/default.settings.php

--- a/sites/default/default.settings.php
+++ b/sites/default/default.settings.php

Erm. I'm pretty sure none of this has anything to do with this patch. :) Needs a re-roll?

Re-rolled!

Changes from #22:

• Rerolled against HEAD; patch no longer applied
• Fixed a few css coding standards along the way, see interdiff-22-23.txt

Re-rolled.

29: seven-layout-styles-2017257-28.patch queued for re-testing.

Recently commited install-page.html.twig #2041793: install-page.html.twig markup and CSS adds one element with l-container class around all 'regions'.
This patch adds the class both inside #branding but on #page element. Is this made like that on purpose? Looks somewhat inconsistent.

It does not apply anymore too. I will re roll this in couple of hours.

First a re-roll of #37 just to have a starting point... and now I'll look what can be improved inline with #2041793: install-page.html.twig markup and CSS.

Small fix.

After looking through the code I think it's the #2041793: install-page.html.twig markup and CSS that will need updating after this gets in not the other way around.

The install page css uses l-container class for more than just layout (background: #fff;) and duplicates it's layout rules.
So the current <div class="l-container"> in install-page.html.twig could probably become <div id="page" class="l-container"> and css in install-page.css will be simplified too.

This fill need a small follow up issue but let's get this one in first.

Failing test fix added.

I've added #page padding back (it was removed in #41). This was breaking other pages too but you might have not seen it if you didn't open any page with secondary tabs.
The reason why this sometimes was not seen is #console which - at least in Chrome - was making top padding on some pages.

Changing #page to .page (or just moving all the styling to .page but leaving #page id in html?) might be a good idea but for a follow-up issue? :)
It's not only #page but #branding, #footer and more.

The patch looks good, applies correctly and needs cache clearing if anyone is testing too.

I think this could be commited and then a follow up applied to other places like node add/edit form, blocks layout and so on.
This should move forward so changing status:)

+++ b/core/modules/system/system.admin.inc
@@ -177,7 +177,7 @@ function theme_admin_page($variables) {
-        $block['position'] = ++$stripe % 2 ? 'left' : 'right';
+        $block['position'] = ++$stripe % 2 ? 'boob' : 'right';

Is this some sort of new-age themer lingo? :-) Otherwise I don't think this is correct.

Haha, nice catch :D

No more boob.

Re-rolled and fixed some minor things.

Not sure about leaving $stripe variable and $block['position'] array in template_preprocess_admin_page() function, but if I remove it it breaks the two column layout in admin/config. Any ideas?

#59 patch applies fine.

There's still some unnecessary left and right margins on #page - around line 435-450 in style.css.

Patch #59 no longer applies, this got in: #2169765: Redesign update.php to be more consistent with the installation process (commitdiff)

Taking up for re-roll.

Re-rolling the patch.

Re-rolling the patch.

This looks RTBC. Nice and simple :)

I'll make a separate issue for my comment from #62.

+++ b/core/themes/seven/css/layout.css
@@ -0,0 +1,35 @@
+  .l-column + .l-column {
+    padding-left: 10px;
+  }
+  .l-column.half {
+    width: 50%;
+  }
+  .l-column.quarter {
+    width: 25%;
+  }
+  .l-column.three-quarter {
+    width: 75%;
+  }

To my knowledge, you can't name a class or an ID that starts with a number.

http://www.markinns.com/articles/full/using_numbers_as_css_class_or_id_v...

Oh is that an L nevermind.

This patch removes generic styles from system.admin.css.

Therefore, we need screenshots of affected output in Stark (and possibly also Bartik).

I'm on it. I'll make some screenshots of config page, blocks page and node edit page.

Since this patch removes from system.admin.css mostly div.admin .left and div.admin .right (and a top padding for div.admin) the only page that changes is admin/config.

All .l-... classes need to be converted to .layout-.. classes to match the policy change in [policy, no patch] Use layout- prefixes instead of l- in CSS Coding Standards

Sprinting on this

Changed classes to use "layout-"

Couple of issues I hit in review:

• layout.css is in the wrong place as set in seven.info.yml
• .layout-colum + .layout-column was broken

Attached fixes those.

Just looking over that patch again and wondering about usage of px vs. em. Should we make one last change?

.layout-column + .layout-column {
  padding-left: 1em;
}

Just playing around with adjusting the base font-size in seven.base.css it’s a very minor difference between 10px and 1em, though I might prefer the proportional measurement. It doesn't seem important enough to hold back this last patch unless we have consensus to focus on proportional vs. fixed units as a rule.

Proportional units are in my opinion more future-proof and device agnostic.

Hey, folks. Really sorry about how long this sat there. My post-DrupalCon life got a bit hectic. :(

Aside from unit concerns mentioned above, the only part here that looks a little questionable to me is:

+++ b/core/modules/system/templates/admin-page.html.twig
@@ -16,8 +16,8 @@
-  {% for position, container in containers %}
-    <div class="{{ position }} clearfix">
+  {% for container in containers %}
+    <div class="layout-column half clearfix">

because "half" seems pretty prescriptive of how this is going to be displayed, for a core template.

OTOH, I think there's a lot of value in splitting off the layout-specific stuff into a separate CSS that can be overridden. We can probably go with this for now and make further tweaks in follow-up issues.

Committed and pushed to 8.x. Thanks!

There should be a third which fixes poor minimal admin pages becoming single column .

That was a bug report...

http://i.imgur.com/LkPY9H2.png that's admin/config and should be two columns.