Remove all visual from stark

there removed the test if the css file was there, cause its not ;)

I agree with the proposal, and the patch looks right to me.

This would be useful/open the door to:

• Troubleshoot Core's CSS.
• Keep an eye on Core and Contrib's CSS over time, perhaps maintain some analytics on it.
• Automating CSS analyzing tools on Core.
• Troubleshoot Contrib's CSS.

I would set this to RTBC, but would like to hear some more opinions on it =)

@maneul scopecreep alarm ;)
BUT as soon as were done with banana #2 (cleanup of all classes out of core & only in classy) then we should begin to look at cores css, at that point we know whats actually really needed for drupal core to work (my bet is a all .js-prefixed & accessability classes)

I would suggest a working order like this:
1. split core css up in [module].admin.css, [module].theme.css & [module].module.css The state of core's css is that the files might are there but the split is not, module have colors in it etc. So a cleanup there is needed

2. Figure out if the css actually should live in bartik or seven

3. lintrap everything in the future + have a team of frontend marines, that can smacss down on modules that dont follow the standards ;)

All of this is think should be discussed in a banana meta#3 , after class cleanup & file organization in classy

Rerolled. Agree with Morten that we should avoid scope creep and work through Manuel's comments in existing (or new) metas.

This looks really good, it's nice to see a clean version of Stark :)

I think we should replace the screenshot as well but because of #2471611: Create HiDPI ready version of theme screenshots we can postpone that task.

Check out the new clean Stark theme:

Screenshot of a fresh Drupal site with patch applied:

I can imagine that people who want to start with a new theme are using Stark and it might be a problem to have no single CSS file there.

yannickoo - nope - people that start a theme outta stark expect NO classes no markup no nothing, else they will use classy, thats the default markup
So its a problem if theres css there!

This is a general misunderstanding of what stark is vs classy & one of the reasons i wanted it to have another name.

bte the frontend liberation army even wanna remove all the markup tags & all css files (but were gonna do that in contrib instead)

Yep this is correct inline with the grand banana plan. I manually tested the patch and it worked with no errors. Go team!

+++ b/core/themes/stark/stark.info.yml
@@ -1,10 +1,6 @@
-stylesheets-remove:
-  - normalize.css

I remember discussing this on another issue and someone being very vociferous about stark not having this css file added. Perhaps normalize should be added to classy. But that is a separate issue.

See #2100571: Do not add normalize.css to stark

I created a follow up to remove this line from Stark: #2472431: Do not load normalize.css in all themes, load it in Classy

+++ b/core/themes/stark/stark.info.yml
@@ -1,10 +1,6 @@
-stylesheets-remove:
-  - normalize.css

We should undo this line removal here, because we don't want to load normalize.css in Stark. We can remove these lines again in the followup

I did this :)

Nice! Thanks.

This does seem to restore my previous understanding of the Stark theme.

However, it does undo an explicit choice made earlier in the release cycle to ensure that all core themes were responsive #1322794: Make Stark use a responsive layout.

Since JohnAlbin pushed for that, and I don't see him in here, assigning to him for review.

*Technically* Stark is responsive even now. It's just not pretty or laid out at all. Since this issue was started over a year after JohnAlbin's issue was wrapped, I think we're okay in how our needs/requirements shifted over time. If it helps, most of what had been done in #1322794: Make Stark use a responsive layout was just copied into Classy, I believe.

Re #28: I can totally agree on that!

Yeah, responsive and mobile first (and last) ;)

In Drupal 7, Stark was "no CSS except a simple layout". In D8, it appears that file has become a dumping ground for anything that module developers think needs to go in Stark: responsive image styles, responsive tables, HTML5 elements.

Okay, I'd rather Stark was "mobile first" (i.e. "mobile only") then what it is now. I'm fine with removing all the existing CSS in Stark. And we've discussed doing this at previous Drupalcons.

But, to be clear, if Stark is to be a window into what core modules provide, it should not be altering the CSS in any way. And, after apply this patch, Stark is still removing normalize.css from core. Let's fix that too. Delete these lines:

stylesheets-remove:
- normalize.css

I'm not so sure about removing that assert in that test. The point of those is to verify that the theme is actually loading by looking for one of its assets. The layout file probably needs to be left, but empty, or find something else to check. Left-but-empty is what we were stuck with for Classy, until we find a better solution. In Stark's case though it shouldn't interfere with anything since it isn't being used as base them.

Re #31: What do you think about #2472431: Do not load normalize.css in all themes, load it in Classy then?

and this is why i wanted yo have another name for the "drupalcore theme"
So we didnt get into confusions about what is what and why its called what it is + all the history that stark have, not to mention a community we have to explain why stark is not stark anymore
#2425715: Rename Stark to Vanilla

Removing the normalize.css override per suggestion in #31, since removing it in this patch does make a bit more philosophical sense than waiting for #2472431: Do not load normalize.css in all themes, load it in Classy.

The layout file probably needs to be left, but empty, or find something else to check.

Here's a patch that adds the layout file back, left empty.

@sqndr Adding interdiff.txt instead

Hi - nice work everyone.

@sqndr Forgive me if you know this already, but if you name the interdiffs with a .txt extension, they won't be sent for testing.

@aliyakhan The file in #41 may have been corrupted. It could be me, but it does not seem to be a standard interdiff. Could you post a new one?

@cilefen: Yeah, I know. Was just a mistake I made ;)

added a new one.

@sqndr #45 does it look fine to you?

@aliyakhan: Yeah, sure … it's the same as mine except for the filename. We just need someone to review the patch from #36 right now.

I dont think we need a stark.libraries.yml file now since we have to remove all styles from stark. we have to make it very simple and clean as per code and file structure.

@sqndr @lewis any thoughts about this ?

It seems so that we need the layout.css file in the stark for testing purposes. These tests couldn't be written for classy since its a base theme for Bartik and Seven but maybe this could be done using Seven?

@Manjit.Singh (#48): @lauriii (#50) is right, see also #32 from David. I added the empty layout.css back and added it into the *.libraries.yml for testing purposes.

I will take a look if this could be changed to base on a testing theme

@lauriii Are you still working on this?

Sorry I forgot this. I will be working today on issues I've forgotten to work on so maybe I will fix this too. If not I'll unassign myself :)

Created tests. For some reason interdiff didn't want to create interdiff for this :/

Rerolled & fixed tests

Empty patch?

Oops!

+++ b/core/modules/system/tests/themes/test_theme/test_theme.libraries.yml
--- /dev/null
+++ b/core/modules/toolbar/src/Tests/ToolbarAdminMenuTest.php.orig

It looks like this patch accidentally includes tests from another patch?

Oopsie again

Maybe finally. Don't know where to create interdiff because last few patches were completely messed up so I didn't create one.

+++ b/core/themes/stark/stark.info.yml
+++ b/core/themes/stark/stark.info.yml
@@ -4,7 +4,3 @@ description: 'An intentionally plain theme with almost no styling to demonstrate

Seems like we can remove the "almost" now :-)

here we go removed almost ;)

Now its failing because last patch removed the libraries.yml file for the test theme

somebody poisened my diff command ...

Thanks @mortendk!

I found two more things to complain about (sorry :-()

+++ b/core/themes/stark/stark.info.yml
@@ -1,10 +1,6 @@
diff --git a/stark-interdiff.txt b/stark-interdiff.txt

diff --git a/stark-interdiff.txt b/stark-interdiff.txt
new file mode 100644

new file mode 100644
index 0000000..e69de29

This should not be in the patch.

Also, shouldn't we be updating the screenshot of Stark? I saw this being discussed above, but I didn't really see a resolution to that and I also don't see it in the patch.

nope let's take the whole screenshot in a follow up issue else this will be so bikesheeded that we can park all of Copenhagen & amsterdams bikes in this issue ;)

I think we need to take the screenshot since there is visual changes.

eeem were talking about the screenshot.png file

screenshots for the theme changes make little sense atm ;)

Monday morning, sorry :D Please make sure they follow the same schema as the new screenshots in #2471611: Create HiDPI ready version of theme screenshots . This was a complicated process during the Montpellier sprint.

looked at a page & nope no screenshots needed as theres not a real change in style:

original:

post patch "design":

im setting this to RTBC

When I remember this correctly we already applied the patch from this issue here when creating screenshots in #2471611: Create HiDPI ready version of theme screenshots .

I agree that stark should be a pure window onto core and module's css and html. With this change we achieve that. Also I think not removing normalize.css is the way to go to. Then if stuff is broken in stark - it is not broken in stark - it is broken in core or the module.

This change is only CSS in the theme layer and therefore is permitted in beta. Committed b1ffbb0 and pushed to 8.0.x. Thanks!

Stark's only "visible action" being the addition of a layout.css that is empty, is a bit puzzling...

From the comments above it seems it was left there only for testing purpose ? If so, maybe a comment in the file would be helpful ?