Problem/Motivation

The compiled version of any file that imports variables.pcss.css includes a contextless block of unhelpful comments from variables.pcss.css.
This is what gets added to dozens of files:

:root {
  /*
   * Color Palette.
   */
  /* Secondary. */
  /* Variations. */ /* 5% darker than base. */ /* 10% darker than base. */ /* 10% darker than base. */ /* 20% darker than base. */ /* 5% darker than base. */ /* 10% darker than base. */ /* 5% darker than base. */ /* 10% darker than base. */ /* 5% darker than base. */ /* 10% darker than base. */
  /*
   * Base.
   */
  /*
   * Typography.
   */ /* 1rem = 16px if font root is 100% ands browser defaults are used. */ /* ~32px */ /* ~29px */ /* ~26px */ /* ~23px */ /* ~20px */ /* 18px */ /* ~14px */ /* ~13px */ /* ~11px */
  /**
   * Spaces.
   */ /* 3 * 16px = 48px */ /* 1.5 * 16px = 24px */ /* 1 * 16px = 16px */ /* 0.75 * 16px = 12px */ /* 0.5 * 16px = 8px */
  /*
   * Common.
   */
  /*
   * Inputs.
   */ /* Absolute zero with opacity. */ /* Davy's gray with 0.6 opacity. */ /* Light gray with 0.3 opacity on white bg. */ /* Old silver with 0.5 opacity on white bg. */ /* (1/8)em ~ 2px */ /* (1/16)em ~ 1px */ /* Font size is too big to use 1rem for extrasmall line-height */ /* 7px inside the form element label. */ /* 8px with the checkbox width of 19px */
  /*
   * Details.
   */
  /**
   * Buttons.
   */
  /**
   * jQuery.UI dropdown.
   */ /* Light gray with 0.8 opacity. */ /* Text color with 0.1 opacity. */
  /**
   * jQuery.UI dialog.
   */
  /**
   * Progress bar.
   */
  /**
   * Tabledrag icon size.
   */ /* 17px */
  /**
   * Ajax progress.
   */
  /**
   * Breadcrumb.
   */
}

This adds unnecessary size to the files, it's confusing, and it makes patch review more difficult. No benefit in having it, but plenty of drawbacks.

Steps to reproduce

Proposed resolution

Modify the css build process to omit these comments from compiled files that import variables.pcss.css.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bnjmnm created an issue. See original summary.

bnjmnm’s picture

This patch is almost 100k. Aside from the 19 lines added to compile.js, the rest of the patch is removing >90k worth of comments that don't need to be there.

lauriii’s picture

We should make sure this works with #3170864: Add postcss-preset-env.

bnjmnm’s picture

I tested this with the most recent patch from #3170864: Add postcss-preset-env. There's no conflict, which expected as this patch adds a plugin to postcssImport, a separate process that occurs before postcssPresetEnv/postCssCustomProperties, and there's nothing in the comments removed in this patch that would impact postcssPresetEnv/postCssCustomProperties.

longwave’s picture

Status: Needs review » Reviewed & tested by the community

I checked through all the CSS files in the diff and there are only comments removed, we should definitely get rid of this cruft, and the question in #3 is resolved so this is RTBC.

lauriii’s picture

Version: 9.1.x-dev » 9.0.x-dev

Committed f4c0228 and pushed to 9.1.x. Thanks!

Leaving open for backport to 9.0.x and 8.9.x.

  • lauriii committed f4c0228 on 9.1.x
    Issue #3171366 by bnjmnm: Comments from variables.pcss.css create...
bnjmnm’s picture

  • lauriii committed 55bc8f6 on 9.0.x
    Issue #3171366 by bnjmnm: Comments from variables.pcss.css create...

  • lauriii committed 9d15696 on 8.9.x
    Issue #3171366 by bnjmnm: Comments from variables.pcss.css create...
lauriii’s picture

Version: 9.0.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Fixed

Committed to 9.0.x and 8.9.x as well. Thanks!

Status: Fixed » Closed (fixed)

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