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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rteijeiro’s picture

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

It looks great!

LewisNyman’s picture

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

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

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

kandra’s picture

FileSize
340.37 KB
139.24 KB
340.83 KB
139.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

*highfive to kandra for the screenshots*

LewisNyman’s picture

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

Issue tags: -Needs screenshots

Status: Reviewed & tested by the community » Needs work

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

mortendk’s picture

Status: Needs work » Needs review

cone on testbot

mortendk’s picture

FileSize
461 bytes

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

mortendk’s picture

Status: Needs review » Reviewed & tested by the community

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

alexpott’s picture

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

we should use:

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

as we will remove all .odd / .even classes

aliyakhan’s picture

FileSize
417 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

Status: Needs work » Needs review
mortendk’s picture

Status: Needs review » Reviewed & tested by the community

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

webchick’s picture

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.