🚧 Big CSS Cleanup - #4170
Conversation
c5f64c9 to
c7d16ee
Compare
Adjusts linting rules to accommodate the CSS modernization effort. Pins stylelint to 15.11.0 and adds stylelint-config-standard-scss. Adds .stylelintignore for developer portal stylesheets. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replaces 115+ hardcoded color definitions with 13 SCSS variables mapped to Patternfly 4 CSS custom properties. Extracts white-box-shadow mixin into shared utils.scss module. Updates all consumers to use new import paths and surviving variable names. Removes dead selectors and vendor-prefixed mixins. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Deletes 7 unused stylesheet partials that contained dead code or styles now handled by Patternfly components. Removes corresponding @forward references from _theme.scss. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extracts body min-width to new provider.scss using Patternfly breakpoint variable. Removes .Main wrapper class from provider layout and compensates spacing through footer margin utility class. Updates Slim syntax to use explicit class attributes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replaces 6 deleted CMS SCSS partials with 3 new standalone stylesheets loaded on-demand. Adopts Patternfly tabs, drag-drop, table, and hint components for CMS UI. Improves drag-and-drop behavior with proper Patternfly state classes. Moves CMS styles from global theme to view-level loading. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replaces 5 separate apiconfig SCSS partials with one integration.scss loaded on-demand from integration views. Removes custom PromoteButton class in favor of Patternfly button classes. Moves styles from global theme to view-level loading. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Uses pf-c-banner for the Red Hat account verification notice. Moves text to i18n locale file for proper translation support. Loads banner CSS on-demand via webpack pack instead of global theme. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replaces StatsCSVLink custom CSS classes with Patternfly button and flex layout utilities in stats views. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wraps javascript/stylesheet pack tags in content_for blocks for proper placement in HTML head. Fixes missing do keywords in content_for calls. Replaces InfoBox custom component with Patternfly cards. Extracts pf_addons as standalone pack. Cleans up JS/CSS pack tag separation. Adds i18n keys for Braintree payment gateway views. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Removes deprecated calculateRem and font-size SCSS functions. Hardcodes wizard-specific color values to decouple from the main color system. Consolidates feedback component styles and removes duplicate keyframes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c7d16ee to
0243087
Compare
0243087 to
e12f200
Compare
akostadinov
left a comment
There was a problem hiding this comment.
auto-approved based on added/removed lines ratio!
|
This PR is stale because it has not received activity for more than 30 days. Remove stale label or comment or this will be closed in 15 days. |
CSS Cleanup Review — Validated FindingsReviewed all 138 changed files for backward compatibility risks and other issues. Each finding below was verified against the actual codebase on the PR branch. Actionable (see inline comments):
Informational (no action needed):
Original review had 18 inline comments. After validating each claim against the codebase, 11 were deleted as false positives or non-actionable. 7 remain. |
| $delete-background-color: $error-background-color; | ||
| $warning-color: $tahitiGold; | ||
| $body-background: white; | ||
| $border-color: var(--pf-global--palette--black-300); |
There was a problem hiding this comment.
Low risk — 95 SCSS variables removed, but all internal references cleaned up.
95 SCSS variables were removed and 12 redefined to use PF CSS custom properties. The PR has cleaned up all internal references across 64 SCSS files — zero dangling references remain on the PR branch. No Sass color functions (darken(), lighten(), etc.) are called on the redefined variables.
The only remaining risk is for downstream forks or out-of-tree SCSS that imports these variables by name. Within this repo the migration is clean.
| end | ||
| end | ||
|
|
||
| def pf_link_to(label, url, options = {}) |
There was a problem hiding this comment.
Minor — pf_link_to moved to PatternflyComponentsHelper with compatible signature.
The method is removed here and recreated in PatternflyComponentsHelper. The signatures are positionally identical — all 13 callers are compatible without changes. No name collision since both changes are in the same PR.
Two minor behavioral differences:
- New version always adds
type=buttonto the<a>tag (PF convention). - String interpolation for class merging leaves a trailing space when no custom class is passed (cosmetic).
| variant = html_options.delete(:variant) || :link | ||
| html_options[:class] = "pf-c-button pf-m-#{variant} #{html_options.delete(:class)}" | ||
| html_options[:type] = :button | ||
|
|
There was a problem hiding this comment.
type: :button on <a> tag diverges from PF4 convention.
PatternFly 4 itself explicitly strips type from anchor-rendered buttons — Button.tsx (line 166) sets type to null when the element is not a <button>, and the PF4 HTML docs show anchor-based buttons with no type attribute.
Per the HTML spec, type on <a> specifies the MIME type of the linked resource (e.g. application/pdf), so type="button" is semantically nonsensical (not a valid MIME type). Browsers silently ignore it, so it's harmless in practice, but it produces technically incorrect HTML and diverges from PF4's own pattern.
Consider removing the type assignment from pf_link_to, since link_to always produces <a> (never <button>).
| $color-text-primary: $color-grey-1; | ||
| $color-text-secondary: $color-grey-2; | ||
| $color-text-tertiary: $color-grey-3; | ||
| $color-border: $color-grey-4; |
There was a problem hiding this comment.
Bug — double semicolon. SCSS will silently accept this, but it's a typo.
$color-brand-primary: #ff7314;;| } | ||
|
|
||
| /* Stats Chart Component */ | ||
| $stats-chart-border-color: $border-color; |
There was a problem hiding this comment.
Visual regression — tooltip shadow will be dramatically darker.
$stats-chart-tooltip-shadow changed from $box-shadow-color (which was rgba($font-color, .1) — 10% transparent) to var(--pf-global--palette--black-800) (near-opaque #3c3f42).
On line 154 this is used directly in:
box-shadow: 0 0 line-height-times(.5) $stats-chart-tooltip-shadow;This will produce a very harsh/dark shadow. Consider using a PF box-shadow variable (--pf-global--BoxShadow--lg) or wrapping in rgba() to preserve the intended transparency.
| list-style: none; } | ||
|
|
||
| .apidocs-param-tips { | ||
| z-index: 9999; /* !!!!!! */ |
There was a problem hiding this comment.
Debug marker and excessive z-index.
- The
/* !!!!!! */comment looks like a debug/temporary marker that shouldn't ship. z-index: 9999is disproportionately high — nearby elements in active-docs usez-index: 1000. Something like1001would be sufficient for a tooltip overlay and more consistent with the rest of the codebase.
| $codemirror-bg-readonly: $codemirror-bg; | ||
| $codemirror-color-readonly: $codemirror-color; | ||
| $codemirror-cursor-color: $highlight-color; | ||
| $orange300: var(--pf-global--palette--orange-300); |
There was a problem hiding this comment.
Minor — CodeMirror theme will look slightly different (intentional alignment).
The old file had hardcoded color copies that were already stale (didn't match _colors.scss on master). The PR correctly aligns with PF CSS custom properties.
Most color deltas are imperceptible (grays differ by 1-5 RGB units). Two visible changes:
- Blue (
#0088ce→#0066cc): more saturated, less cyan. The old value was already wrong vs_colors.scss. cm-specialwarning color (#ec7a08→#f0ab00): orange shifts to amber/gold. This is a token remapping from$tahitiGoldto PF--pf-global--warning-color--100.
Worth a visual check of the CodeMirror editor, but this is intentional alignment, not an accidental regression.
🚧 WIP do not review 🚧
THREESCALE-12047: Clean up unused CSS
What does this do: