Problem/Motivation

See: #1190252: [573] Use csslint as a weapon to beat the crappy CSS out of Drupal core

[13:32:44] 1 error found in /Library/WebServer/Documents/core/modules/node/css/node.module.css
[13:32:44] [L24:C5] Using width with padding-right can sometimes make elements larger than you expect. Don't use width or height when using padding or border. (box-model)
[13:32:44] 2 errors found in /Library/WebServer/Documents/core/modules/node/css/node.preview.css
[13:32:44] [L10:C3] Using width with padding can sometimes make elements larger than you expect. Don't use width or height when using padding or border. (box-model)
[13:32:44] [L19:C18] Values of 0 shouldn't have units specified. You don't need to specify units when a value is 0. (zero-units)

Proposed resolution

Fix the errors, review the patch.

Remaining tasks

Write a patch to fix the suggestions
Run CSSlint against the new CSS

User interface changes

None

API changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

LewisNyman’s picture

Issue tags: +Novice
Manjit.Singh’s picture

1. Removing padding from core/modules/node/css/node.module.css
2. Minimize the width of .node-preview-container.

Little bit confuse in node preview bar... We have already added css in bartik then why we adding the same in node module.

1. CSS in node module

/**
 * @file
 * Styles for node preview page.
 */

.node-preview-container {
  position: fixed;
  z-index: 499;
  width: 100%;
  padding: 10px;
}

2. CSS in Bartik /core/themes/bartik/css/components/node-preview.css

.node-preview-container {
  background: #d1e8f5;
  background-image: -webkit-linear-gradient(top, #d1e8f5, #d3e8f4);
  background-image: linear-gradient(to bottom, #d1e8f5, #d3e8f4);
  font-family: Arial, sans-serif;
  box-shadow: 0 1px 3px 1px rgba(0, 0, 0, 0.3333);
  position: fixed;
  z-index: 499;
  width: 100%;
  padding: 10px;
}

Lewis can you please check it.

LewisNyman’s picture

Little bit confuse in node preview bar... We have already added css in bartik then why we adding the same in node module.

I guess someone just copied the CSS and added some styles? We should clean this up in #2384169: The node preview bar is not usable without Bartik

LewisNyman’s picture

Component: locale.module » node system
Status: Needs review » Needs work
  1. +++ b/core/modules/node/css/node.module.css
    @@ -20,8 +20,8 @@
    -    width: 65%;
    -    padding-right: 2em; /* LTR */
    +    width: 61%;
    +    margin-right: 2em; /* LTR */
    

    I think that this will change the layout of the page and produce inconsistent results at different screen sizes. I think we can just use box-sizing: border-box here instead https://github.com/CSSLint/csslint/wiki/Beware-of-box-model-size

  2. +++ b/core/modules/node/css/node.preview.css
    @@ -6,7 +6,7 @@
    -  width: 100%;
    +  width: 98%;
    

    Same situation here

Manjit.Singh’s picture

Status: Needs work » Needs review
Related issues: +#2384169: The node preview bar is not usable without Bartik
FileSize
897 bytes

@lewis Adding box-sizing as suggested by you

Also regarding cleanup css for node-preview bar, we will follow #2384169: The node preview bar is not usable without Bartik this only.

LewisNyman’s picture

Status: Needs review » Reviewed & tested by the community

Ok I manually tested the patch for any visual regressions, I can also confirm that there aren't any more CSSlint errors in these files. Thanks!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

CSS only change - passes beta evaluation. Committed 9a36faf and pushed to 8.0.x. Thanks!

  • alexpott committed 9a36faf on 8.0.x
    Issue #2485505 by Manjit.Singh, LewisNyman: Remove CSSlint errors from...

Status: Fixed » Closed (fixed)

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