Rename layout classes to follow the layout style naming convention

Adding tags

Updating issue Summary

Why are we not using the following?

.layout-sidebar-first
.layout-sidebar-last
.layout-no-sidebars
.layout-two-sidebars

I just think it's a bit idiosyncratic.

Drupal has long espoused an admirable coding standard to avoid abbreviating the names of things to avoid encoding their meaning. I don't follow SMACSS per se but after reading the linked article it seems that I do indeed do everything described in my regular workflow (except use abbreviated classnames).

In the comments of the linked article, Jonathon Snook uses the following example when responding to a question (indentation/spacing mine):

@media screen and (min-width: 800px) {
  .layout-contact .content { float: left; }
  .layout-contact .sidebar { float: right; }
}

Not having read through the whole SMACSS documentation, I think the general guidelines it presents makes a lot of sense and I'm all for adopting these patterns within Drupal but I do think the smaller details such as adopting the specific naming of things are much less important.

Does anyone else have strong feelings on this?

@pixelwhip, @psynaptic,
I think there is no difference from using l- or layout-
The naming conventions will take something like 10 line of documentation, and every themer can read them without any problem, before starting work.
I personally think we should keep the prefixes short, so it takes a little bit less time to understand what does a specific style do. And less time to write and mistake chance is lower: for foreigner not knowing English so good, 'layout' can be spelled with a mistake, but l- is more likely to be ok :)

All class names are proper and work fine. +1

For anyone else who did a double-take thinking "No way are those valid class names," those classes start with the lowercase letter L, not the numeral 1.

If someone can confirm/deny whether or not ".l" specifically is part of the recommendations at http://smacss.com/book/type-layout, that would be great. From my interpretation, it sounds like the method described there is part of the spec, not the specific, obtuse "l-xxx" class names which have screwed up at least 3 people reading this issue (myself included). I really disilke code standards guidelines like "don't ever abbreviate names of things" that have "oh, except for..." after them. :\ Makes it really difficult for reviewers and contributors alike.

Not marking "needs work," but marking "needs review" and assigning to John in case he can shed further light on this. I would much prefer to spell out "layout" here so it doesn't screw up anywhere else, as long as people familiar with SMACSS won't find this a weird Drupalism.

In the SMACSS book Snookca does use ".l-" for layout, but that doesn't mean we need to.

In his talk at DrupalconPDX, the question of naming as it applies to classes and so on was put forward during Q&A, and his response was (and I'm paraphrasing) "Find conventions that work for your team."

I don't think that adding ".l-" is an improvement over what we have now. I am in favor of ".layout-". I realize that there could be the introduction of spelling error, brought up in #7, but at least a full word is translatable and adds more context than just a letter.

1: drupal-rename-layout-classes-1054616-1.patch queued for re-testing.

I prefer to use more meaningful names. I think it's hard to argue for '.l-' because it's so minimal. This is coming from someone who didn't realise what the l() function in core did for ages :P

Considering how old the previous patch way it seems easier just to rewrite it from scratch, please check to see if I've missed them somewhere.

15: drupal-rename-layout-classes-1054616-15.patch queued for re-testing.

@Abby805, related #2093341: Change the machine name/title of the sidebar regions to something more semantic

This issue has kind of stalled. The best place to debate the CSS standards is here and they have already been RTBC'd. I think we should follow them for the time being and maybe take the '.l-' vs '.layout-' discussion over there.

15: drupal-rename-layout-classes-1054616-15.patch queued for re-testing.

Here's a re-roll of #15. I've reverted to l- prefixes, following our standards. I hope there's still room to change the standards and adapt CSS later - but for this issue I'm sticking with l-.

24: drupal-rename_layout_classes-1054616-24.patch queued for re-testing.

Almost all of the comments above endorse the usage of layout- rather than l- in class names. So if we're going to introduce this change, why not just make the change to what most people would like to see?

@mdrummond I'd rather open a new issue for that.

Almost all of the comments above endorse the usage of layout- rather than l- in class names. So if we're going to introduce this change, why not just make the change to what most people would like to see?

Everyone who was involved in the creation of the CSS guidelines is not following this issue. It's not immediately clear that we are making a policy change in this issue either. I'd rather we didn't make the decision here.

Agreed. Here's a policy issue: #2207647: [policy, no patch] Use layout- prefixes instead of l- in CSS Coding Standards

1: drupal-rename-layout-classes-1054616-1.patch queued for re-testing.

24: drupal-rename_layout_classes-1054616-24.patch queued for re-testing.
this patch is needed retest

#2207647: [policy, no patch] Use layout- prefixes instead of l- in CSS Coding Standards went through deciding to use "layout-" so here's the #24 patch back to using that.

Applying patch with reroll and required changes which fixes this issue. Please review.

37: drupal8-rename_layout_classes_to_follow_the_layout_style_naming_convention-1912610-37.patch queued for re-testing.

Submitting patch again.

Updating classes in issue to follow naming.

patch fails to install on simplytest.me

Patch fails to install

The patch no longer applies so a reroll is needed.

Related: #2234331: Change the body classes to follow Drupal 8 CSS standards proposing to remove sidebar classes altogether

I'll take on this issue as my first patch!

Rerolled.

Someone should look at the uses of l-sidebar in these files and see if it is changing those is relevant to this issue:
core/modules/system/templates/install-page.html.twig
core/themes/seven/maintenance-page.css
core/themes/seven/templates/install-page.html.twig
core/themes/seven/templates/maintenance-page.html.twig

If no one looks at by next week, I'll take a closer look at it.

Needs work as mentioned in #53.

I'll take a peek at this later today.

All the files mentioned in #53 need to be edited to reflect this naming convention change. Patch to come.

Ugh, sorry...ignore this patch-- it's jacked up. Fixing now.

OK, sorry, this is my first patch. Here's the new-and-improved version.

The latest patch is basically an interdiff from #53 which is good to have. We also need the complete patch diffed from 8.x head.

The new changes look good to me.

Ahhh, OK. Sorry, I'm still learnin'!

Bryan

Hmm...I get this error when I try applying the patch from #53:

error: core/modules/block/custom_block/src/Tests/CustomBlockListTest.php: No such file or directory

I even re-cloned the D8 repo just to make sure I wasn't missing any files, but there's no 'custom_block' directory in /core/modules/block/...what kind of noob mistake am I making?

Thanks!
Bryan

That file got renamed since my patch. So, now it needs a reroll, and then you can apply your patch. See https://drupal.org/patch/reroll.

OK, here is the patch from #53 re-rolled.

Auto-merge, no conflicts.

If the re-rolled patch from #65 passes testing, I'll add my patch from #59.

Whew! OK, here is the patch that combines the re-rolled patch from #65 with the patch I created on comment #59.

Many thanks to lokapujya for his patience and guidance!

Bryan

+++ b/core/modules/system/templates/maintenance-page.html.twig
@@ -64,4 +64,5 @@
+

Un-needed new extra blank line. Other than that, it looks good to me. You can also remove the "Needs reroll" tag.

Please review updated patch as per fixes in #68.

Please note that changes in file core/modules/block_content/src/Tests/BlockContentListTest.php are no longer required as those changes are already in core.

Patch from #67 with extra newline removed.

I went through all the themes, and install.php, and I don't see any layout bugs. I also had a look through the themes and modules to look for remaining .l- classes and I couldn't find any.

Awesome, much better. :) Thanks for all the hard work on this.

Committed and pushed to 8.x. Thanks!