Problem/Motivation

With the cleanup of all core templates and the creation of the Classy theme to be the base theme for Seven and Bartik
Stark should not have any "design"
This would expose what core really outputs by default with no custom CSS over. Any CSS left should be functional.
Stark as a bare metal theme would also serve module developers that want to separate functional CSS from pretty CSS.
It would also be a very easy theme to maintain in core.

Proposed resolution

- Remove any custom CSS from Stark
- Don't let Stark remove any css files outputted from core
- Don't let Stark define any breakpoints (useless without css to leverage it)
- Change the description and readme file of the theme to reflect the change
- Update the screeshot

Files: 
CommentFileSizeAuthor
#82 Home___Stark_and_15_Must-Know_Chrome_DevTools_Tips_and_Tricks___Tutorialzine_and_Employees_on_iTunes.png83.69 KBmortendk
#82 screenshot.png31.25 KBmortendk
#76 remove_all_visual_from-2349711-74.patch8.64 KBb0unty
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 97,045 pass(es). View
#73 remove_all_visual_from-2349711-72.patch8.74 KBmortendk
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 96,515 pass(es). View
#73 stark-interdiff.txt699 bytesmortendk
#70 remove_all_visual_from-2349711-70.patch8.27 KBmortendk
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 96,412 pass(es), 1 fail(s), and 0 exception(s). View
#70 stark-interdiff.txt892 bytesmortendk
#66 remove_all_visual_from-2349711-66.patch8.25 KBlauriii
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 96,255 pass(es). View
#64 interdiff.txt21.25 KBlauriii
#64 remove_all_visual_from-2349711-64.patch22.65 KBlauriii
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View
#60 remove_all_visual_from-2349711-60.patch30.87 KBlauriii
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,684 pass(es). View
#58 remove_all_visual_from-2349711-58.patch0 byteslauriii
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,503 pass(es). View
#55 remove_all_visual_from-2349711-55.patch9.14 KBlauriii
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,543 pass(es), 5 fail(s), and 0 exception(s). View
#36 interdiff.patch1.36 KBsqndr
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch interdiff_42.patch. Unable to apply patch. See the log in the details link for more information. View
#36 stark-remove-visuals-2349711-36.patch4.5 KBsqndr
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,425 pass(es). View

Comments

Jolidog’s picture

Status: Active » Needs review
FileSize
5.04 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,858 pass(es), 3 fail(s), and 0 exception(s). View

Here is the patch.

Jolidog’s picture

Let's see what the test bots have to say...

Status: Needs review » Needs work

The last submitted patch, 1: bare_metal_stark-2349711-1.patch, failed testing.

mortendk’s picture

Issue tags: +classy
mortendk’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
5.31 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,734 pass(es), 1 fail(s), and 0 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 5: bare_metal_stark-2349711-2.patch, failed testing.

mortendk’s picture

Title: Make Stark a bare metal theme » Remove all visual from stark
mortendk’s picture

Status: Needs work » Needs review
FileSize
6.01 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,777 pass(es). View

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

mortendk’s picture

Component: theme system » Stark theme
mortendk’s picture

Issue tags: +frontend
Manuel Garcia’s picture

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 =)

mortendk’s picture

@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

akalata’s picture

Related issues: +#2289511: [meta] Results of Drupalcon Austin's Consensus Banana
FileSize
5.93 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,383 pass(es). View

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

yannickoo’s picture

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:

yannickoo’s picture

Screenshot of a fresh Drupal site with patch applied:

yannickoo’s picture

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.

mortendk’s picture

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)

LewisNyman’s picture

Status: Needs review » Reviewed & tested by the community

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

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ 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.

alexpott’s picture

LewisNyman’s picture

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

akalata’s picture

Status: Needs work » Reviewed & tested by the community
LewisNyman’s picture

Status: Reviewed & tested by the community » Needs work
+++ 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

yannickoo’s picture

Status: Needs work » Needs review
FileSize
570.99 KB

I did this :)

yannickoo’s picture

FileSize
7.44 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,400 pass(es). View
LewisNyman’s picture

Status: Needs review » Reviewed & tested by the community

Nice! Thanks.

webchick’s picture

Assigned: Unassigned » JohnAlbin
Status: Reviewed & tested by the community » Needs review

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.

akalata’s picture

*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.

yannickoo’s picture

Re #28: I can totally agree on that!

Manuel Garcia’s picture

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

JohnAlbin’s picture

Assigned: JohnAlbin » Unassigned
Status: Needs review » Needs work

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

davidhernandez’s picture

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.

yannickoo’s picture

mortendk’s picture

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

akalata’s picture

Status: Needs work » Needs review
FileSize
5.96 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,816 pass(es). View

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.

sqndr’s picture

FileSize
4.5 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,425 pass(es). View
1.36 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch interdiff_42.patch. Unable to apply patch. See the log in the details link for more information. View

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.

Status: Needs review » Needs work

The last submitted patch, 36: interdiff.patch, failed testing.

The last submitted patch, 36: interdiff.patch, failed testing.

sqndr’s picture

sqndr’s picture

Issue tags: +CSS
aliyakhan’s picture

FileSize
2.17 KB

@sqndr Adding interdiff.txt instead

aliyakhan’s picture

Status: Needs work » Needs review
cilefen’s picture

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?

sqndr’s picture

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

aliyakhan’s picture

FileSize
1.85 KB

added a new one.

aliyakhan’s picture

@sqndr #45 does it look fine to you?

sqndr’s picture

@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.

Manjit.Singh’s picture

FileSize
4.88 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,596 pass(es), 1 fail(s), and 0 exception(s). View
663 bytes

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 ?

Status: Needs review » Needs work

The last submitted patch, 48: stark-remove-visuals-2349711-48.patch, failed testing.

lauriii’s picture

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?

sqndr’s picture

@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.

lauriii’s picture

Assigned: Unassigned » lauriii

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

LewisNyman’s picture

@lauriii Are you still working on this?

lauriii’s picture

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 :)

lauriii’s picture

Status: Needs work » Needs review
FileSize
9.14 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,543 pass(es), 5 fail(s), and 0 exception(s). View

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

lauriii’s picture

Assigned: lauriii » Unassigned

Status: Needs review » Needs work

The last submitted patch, 55: remove_all_visual_from-2349711-55.patch, failed testing.

lauriii’s picture

Status: Needs work » Needs review
FileSize
0 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,503 pass(es). View

Rerolled & fixed tests

yannickoo’s picture

Status: Needs review » Needs work

Empty patch?

lauriii’s picture

Status: Needs work » Needs review
FileSize
30.87 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,684 pass(es). View

Oops!

Status: Needs review » Needs work

The last submitted patch, 60: remove_all_visual_from-2349711-60.patch, failed testing.

Status: Needs work » Needs review
LewisNyman’s picture

Status: Needs review » Needs work
+++ 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?

lauriii’s picture

Status: Needs work » Needs review
FileSize
22.65 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View
21.25 KB

Oopsie again

Status: Needs review » Needs work

The last submitted patch, 64: remove_all_visual_from-2349711-64.patch, failed testing.

lauriii’s picture

Status: Needs work » Needs review
FileSize
8.25 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 96,255 pass(es). View

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

Status: Needs review » Needs work

The last submitted patch, 66: remove_all_visual_from-2349711-66.patch, failed testing.

tstoeckler’s picture

+++ 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 :-)

Status: Needs work » Needs review
mortendk’s picture

FileSize
892 bytes
8.27 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 96,412 pass(es), 1 fail(s), and 0 exception(s). View

here we go removed almost ;)

Status: Needs review » Needs work

The last submitted patch, 70: remove_all_visual_from-2349711-70.patch, failed testing.

lauriii’s picture

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

mortendk’s picture

Status: Needs work » Needs review
FileSize
699 bytes
8.74 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 96,515 pass(es). View

somebody poisened my diff command ...

The last submitted patch, 70: remove_all_visual_from-2349711-70.patch, failed testing.

tstoeckler’s picture

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.

b0unty’s picture

FileSize
8.64 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 97,045 pass(es). View

removed the empty interdiff from patch

mortendk’s picture

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 ;)

lauriii’s picture

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

yannickoo’s picture

Issue tags: +Needs screenshots
mortendk’s picture

Issue tags: -Needs screenshots

eeem were talking about the screenshot.png file

screenshots for the theme changes make little sense atm ;)

yannickoo’s picture

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.

mortendk’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
31.25 KB
83.69 KB

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

yannickoo’s picture

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 .

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

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!

  • alexpott committed b1ffbb0 on 8.0.x
    Issue #2349711 by lauriii, mortendk, sqndr, akalata, yannickoo, Manjit....

Status: Fixed » Closed (fixed)

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

yched’s picture

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 ?