Problem/Motivation

core/misc/print.css is failing lintcss test

Proposed resolution

remove unused #menu & #main (not used anymore)
odd & even classes corrected

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#15 csslint-2425013-15.patch417 bytesaliyakhan
#11 csslint-print.diff461 bytesmortendk
#5 new_print2.png139.33 KBkandra
#5 new_print1.png340.83 KBkandra
#5 original_print2.png139.24 KBkandra
#5 original_print.png340.37 KBkandra
#1 print-css.png144.98 KBrteijeiro
csslint-print.diff461 bytesmortendk

Comments

rteijeiro’s picture

rteijeiro commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new print-css.png144.98 KB

It looks great!

lewisnyman’s picture

lewisnyman
he/him
commented
Status: Reviewed & tested by the community » Needs review
Issue tags: +frontend, +Needs screenshots

Given that we are changing table styling, a screenshot with a table in would be nice ;-)

Also, I can see that we are removing the decision to hide the menu, this makes sense, as core shouldn't really be that opinionated about what themes do with their menus when printed.

I wonder how much print CSS we really need to be global across all themes, I know that this issue is about only csslint fixes but it would be nice to clean it up even more in this issue or a follow up.

mortendk’s picture

mortendk commented

yeah print.css should live in the theme, as they normally allready do
I have a sneaky suspision that misc/print.css was added at some point where print stylesheets was the hot stuff, so it got dropped into misc/ to never be heard from again ;)

lewisnyman’s picture

lewisnyman
he/him
commented

I have asked someone to work on the screenshots for tables at Drupalcon

kandra’s picture

kandra commented
StatusFileSize
new original_print.png340.37 KB
new original_print2.png139.24 KB
new new_print1.png340.83 KB
new new_print2.png139.33 KB

Looks good with tables. I'm attaching the original screenshots and how it looks after applying the patch.

Only local images are allowed.

Only local images are allowed.

And after applying the patch

after patch print

after patch print

mortendk’s picture

mortendk commented

*highfive to kandra for the screenshots*

lewisnyman’s picture

lewisnyman
he/him
commented
Status: Needs review » Reviewed & tested by the community

I had a quick chat with Morten in IRC and we agreed that we will create a follow up that will attempt to remove this CSS file completely.

lewisnyman’s picture

lewisnyman
he/him
commented
Issue tags: -Needs screenshots

Status: Reviewed & tested by the community » Needs work

The last submitted patch, csslint-print.diff, failed testing.

mortendk’s picture

mortendk commented
Status: Needs work » Needs review

cone on testbot

mortendk’s picture

mortendk commented
StatusFileSize
new csslint-print.diff461 bytes

reuoloads the same patch as it seems to be gone missing ??

mortendk’s picture

mortendk commented
Status: Needs review » Reviewed & tested by the community

sets back to RTBC based on #7 after testbot got drunk

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/misc/print.css
@@ -14,18 +13,12 @@ th {
-tr.odd {
+.odd {
...
-tr.even {
+.even {

.odd and .even are exceptionally generic - let's leave this alone.

mortendk’s picture

mortendk commented

we should use:

tr:odd { color:gray }
tr:even { color:lightgray }

as we will remove all .odd / .even classes

aliyakhan’s picture

aliyakhan commented
StatusFileSize
new csslint-2425013-15.patch417 bytes

I've replaced it with nth-child
tr.odd {
background-color: #ddd;
}
tr.even {
background-color: #fff;
}

with

tr:nth-child(odd) {
background-color: #ddd;
}
tr:nth-child(even){
background-color: #fff;
}

Also, as per #14 there are no pseudo classes as :odd & :even for now.

aliyakhan’s picture

aliyakhan commented
Status: Needs work » Needs review
mortendk’s picture

mortendk commented
Status: Needs review » Reviewed & tested by the community

@aliyakhan cool this should bring a bit more happiness in the frontend

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 082a601 on 8.0.x 
    Issue #2425013 by kandra, mortendk, aliyakhan, rteijeiro, LewisNyman:...

Status: Fixed » Closed (fixed)

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