Follow-up to #2612618: Add README.txt file to Stable explain its role as a backwards compatibility layer

Current first paragraph of Stable's README:

Stable is a base theme with minimal markup and very few classes. It functions
as a backwards compatibility layer for Drupal 8's core markup and CSS. Stable
allows markup and styling to evolve throughout the life of Drupal 8, while still
providing themes a stable base for the clean, minimal markup provided by core.

Overall to me this first paragraph reads more like an explanation we would give to a core contributor rather than a new themer (for example). It also contradicts the next paragraph with this phrase: "clean, minimal markup provided by core". The next paragraph says "This ensures that changes made to core markup and styling do not affect themes using Stable as a base theme.".

I think we can make it easier to understand. Here's an attempt:

Stable is the default base theme in Drupal 8 which provides minimal markup and very few classes. If you prefer more structure and classes see the Classy base theme (also in Drupal core).

All the backwards compatibility, evolution, etc. stuff could be explained further down I think for those who are interested. I also think we should add a similar README to Classy which points back to Stable.

Comments

Cottser created an issue. See original summary.

jhodgdon’s picture

OK, seems like a good idea.

Minor grammatical/typographical tweak:

Stable is the default base theme in Drupal 8; it provides minimal markup and very few classes.

Hm... do you think it would make sense to say "very few HTML class attributes"? Classes could get confusing for programmers thinking of JS or PHP classes. ???

anil280988’s picture

Status: Active » Needs review
StatusFileSize
new1.88 KB

Hi Cottser,
Agreed that the line contradicts the next paragraph. The line is correct though, so changing the paragraph, among other minor changes.

hussainweb’s picture

StatusFileSize
new1.34 KB
new1.93 KB

I found the second paragraph somewhat confusing. Do you think this is better?

anil280988’s picture

Hi Hussainweb,
Yes it certainly seems better. Do you have any more suggestion to make it more informational.

hussainweb’s picture

I can't think of anything else. Of course, more review might bring up other points. :)

star-szr’s picture

Status: Needs review » Needs work

Thanks!

  1. +++ b/core/themes/stable/README.txt
    @@ -2,23 +2,24 @@
    +Stable is the default base theme in Drupal 8, it provides minimal markup and
    

    Comma should be semicolon per @jhodgdon in #2.

  2. +++ b/core/themes/stable/README.txt
    @@ -2,23 +2,24 @@
    +very few classes. If you prefer more structure and drupal classes see the Classy
    

    What are drupal classes? I'd prefer to go more along the way @jhodgdon outlined in #2. This wording IMO doesn't add clarity or help differentiate from PHP classes.

  3. +++ b/core/themes/stable/README.txt
    @@ -2,23 +2,24 @@
    +in Drupal 8's core markup, Javascript and CSS. It allows the core markup and
    

    Remove the Javascript part, Stable does not contain JS.

anil280988’s picture

Status: Needs work » Needs review
StatusFileSize
new1.91 KB

Hi Cottser,
Made the changes and updated the patch.

davidhernandez’s picture

+++ b/core/themes/stable/README.txt
@@ -2,23 +2,24 @@
+very few classes. If you prefer more structure and drupal classes see the Classy

Just add CSS here, "...very few CSS classes..." If this is primarily for themers, they'll know exactly what that means. I'm not sure "HTML class attributes" helps. It is wordier, and classes themselves are not attributes.

Maybe rewrite the last sentence as "If you prefer more structured markup see the Classy..." The word markup is used in the previous sentence to clarify Stable's purpose, so it would make sense to continue to Classy.

+++ b/core/themes/stable/README.txt
@@ -2,23 +2,24 @@
+Warning: Themes that opt out of using Stable as base theme will need continuous

This needs an article before "base theme". "a" or "the".

I don't agree with the change to the second paragraph. It has become too generalized. The original second sentence could use clarification, but the first sentence was making a point that copies of all templates and CSS are in Stable. I think it is an important point, because that is where we will tell people to look for those files, and this is one of the only instance where a theme actually has copies of everything instead of relying on overrides.

jhodgdon’s picture

Status: Needs review » Needs work

One more thing:

  1. +++ b/core/themes/stable/README.txt
    @@ -2,23 +2,24 @@
    +Stable is the default base theme in Drupal 8; it provides minimal markup and
    

    Let's not say "in drupal 8" here.

    We try not to use the word Drupal explicitly in our docs. And having versions in docs mean that when we get to Drupal 9, a bunch of our docs are wrong/obsolete/need changing.

  2. +++ b/core/themes/stable/README.txt
    @@ -2,23 +2,24 @@
    +in Drupal 8's core markup and CSS. It allows the core markup and styling to
    

    here too

  3. +++ b/core/themes/stable/README.txt
    @@ -2,23 +2,24 @@
    +evolve throughout the life of Drupal 8, without worrying about breaking
    

    here too

anil280988’s picture

Status: Needs work » Needs review
StatusFileSize
new1.97 KB

Hi davidhernandez/jhodgdon,
Implemented the asked changes.

anil280988’s picture

StatusFileSize
new1.75 KB

Adding interdiff file.

star-szr’s picture

Status: Needs review » Needs work

Overall looks pretty good to me at least at a glance, thank you!

One small thing:

+++ b/core/themes/stable/README.txt
@@ -2,23 +2,25 @@
+CSS classes. If you prefer more structure markup see the Classy base theme

structure = structured?

anil280988’s picture

Status: Needs work » Needs review
StatusFileSize
new1.97 KB

Done.

star-szr’s picture

One more thought…

+++ b/core/themes/stable/README.txt
@@ -2,23 +2,25 @@
+of all the Twig templates and CSS files provided by core. It allows the core

"It" is maybe a bit weird here, maybe "Stable" instead of "It"?

davidhernandez’s picture

Looking better. The last sentence of the second paragraph could use some rewording. It just reads a bit odd to me. (Personal opinion maybe)

+++ b/core/themes/stable/README.txt
@@ -2,23 +2,25 @@
+of all the Twig templates and CSS files provided by core. It allows the core
+markup and styling to evolve, without worrying about breaking anything in
+contributed and custom themes.

Maybe "This" instead of "It", since it is a continued thought from the previous sentence, and "the" before core can probably go. How about:

This allows core markup and styling to evolve, without causing problems for contributed or custom themes.
drupal.ninja03’s picture

StatusFileSize
new1.96 KB

Hi Cottser/davidhernandez,
Created patch, with suggested changes.

star-szr’s picture

Status: Needs review » Reviewed & tested by the community

Looks great, we might not want to add the blank line and indent for the base theme: false part but I'll let @jhodgdon weigh in on that part. Otherwise to me this is a big improvement and ready to ship, thanks all!

jhodgdon’s picture

Status: Reviewed & tested by the community » Needs review

Looking great! I have two small suggestions:

  1. +++ b/core/themes/stable/README.txt
    @@ -2,23 +2,25 @@
    +(also in Drupal core).
    

    We don't need this IMO. I think if we were going to point to a non-core theme, we'd be expected to provide a link. The fact that this is in a core README kind of implies by default we're talking about core.

  2. +++ b/core/themes/stable/README.txt
    @@ -2,23 +2,25 @@
     
    

    I like the indent -- I think it makes this more readable -- but I don't think we need the blank line before the base theme: false line.

I'm willing to be overruled on both of these so I've just set this back to Needs review. Thoughts?

star-szr’s picture

Status: Needs review » Needs work

That makes sense, I agree with both of those @jhodgdon :) thanks.

davidhernandez’s picture

+++ b/core/themes/stable/README.txt
@@ -2,23 +2,25 @@
+CSS classes. If you prefer more structured markup see the Classy base theme

Sorry if this is pedantic, but this part, "...the Classy base theme..." doesn't read right to me with an article before Classy. Does it seem odd to anyone else? Maybe remove it and end the sentence on Classy? "If you prefer more structured markup see Classy."

+++ b/core/themes/stable/README.txt
@@ -2,23 +2,25 @@
+in core markup and CSS. If you browse Stable's contents, you will find copies

Change "in" to "to" so it reads "against changes to core markup and CSS."

+++ b/core/themes/stable/README.txt
@@ -2,23 +2,25 @@
+Stable functions as a backwards compatibility layer for themes against changes
+in core markup and CSS. If you browse Stable's contents, you will find copies
+of all the Twig templates and CSS files provided by core. Stable allows core
+markup and styling to evolve, without causing problems for contributed or
+custom themes.

Is it just me or do the first and third sentence essentially say the same thing? I might combine them.

Stable allows core markup and styling to evolve by functioning as a backwards compatibility layer for themes against changes to core markup and CSS. If you browse Stable's contents, you will find copies of all the Twig templates and CSS files provided by core.

Or, going back to my comment in #16, "It" was changed to "Stable" but I think "This" still does a better job making the sentences flow.

Stable functions as a backwards compatibility layer for themes against changes in core markup and CSS. This allows core markup and styling to evolve, without causing problems for contributed or custom themes. If you browse Stable's contents, you will find copies of all the Twig templates and CSS files provided by core.

"This" does not refer to Stable, it refers to its function.

I agree with #19.1 and could go either way on #19.2. I prefer the space, since it makes it a little cleared that we are now demonstrating a piece of code essentially. If we provide a code snippet in a book or on a blog, wouldn't there be a space before the snippet? But that is my opinion. I'm not sure I feel too strong about it either way.

anil280988’s picture

Status: Needs work » Needs review
StatusFileSize
new1.87 KB

Hi jhodgdon/davidhernandez,
Removed blank line before the base theme: false line, as it now look a continuation from last line.
Points from davidhernandez
- I think "...the Classy base theme..." is correct. Ending it with only "Classy" will sound vague.
- Made other changes as suggested.

anil280988’s picture

Any suggestions on this?

anil280988’s picture

Any suggestions on this?

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

I like it! Tentatively marking RTBC; let's leave this for at least a day or two so Cottser and davidhernandez can comment if desired. Thanks!

davidhernandez’s picture

Looks good to me.

star-szr’s picture

+1 :)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed c53c684 and pushed to 8.0.x and 8.1.x. Thanks!

  • alexpott committed 04df441 on 8.1.x
    Issue #2630592 by anil280988, hussainweb, drupal.ninja03, Cottser,...

  • alexpott committed c53c684 on
    Issue #2630592 by anil280988, hussainweb, drupal.ninja03, Cottser,...

Status: Fixed » Closed (fixed)

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