Skip to content

ROX-33794: Add no-PatternFly-utility-layout lint rule with ignores#19608

Open
pedrottimark wants to merge 2 commits intomasterfrom
ROX-33794-pluginLimited-no-PatternFly-utility-layout
Open

ROX-33794: Add no-PatternFly-utility-layout lint rule with ignores#19608
pedrottimark wants to merge 2 commits intomasterfrom
ROX-33794-pluginLimited-no-PatternFly-utility-layout

Conversation

@pedrottimark
Copy link
Contributor

Description

Objectives

  1. Discover whether utility classes (versus props) for layout are needed.
  2. Identify files that have occurrences for potential replacement.

Problem

PatternFly upgrade:

  • Replaces version number in utility classes.
  • Does not replace props of elements.

Therefore, utility classes for layout that are equivalent to props of elements cause more changes files than needed.

Analysis

https://www.patternfly.org/utility-classes/spacing#margin-properties

margin = m|mb|ml|mr|mt|mx|my in table

https://www.patternfly.org/utility-classes/spacing#padding-properties

padding = p|pb|pl|pr|pt|px|py in table

margin_padding = m|mb|ml|mr|mt|mx|my|p|pb|pl|pr|pt|px|py in table

https://www.patternfly.org/utility-classes/spacing#size-values

Find in Files with regular expression enabled

results files pattern
215 117 pf-v6-u-(margin)-[^0]
270 117 pf-v6-u-(padding)-[^0]
485 194 pf-v6-u-(margin_padding)-[^0]
27 21 pf-v6-u-(margin)-0
44 25 pf-v6-u-(padding)-0
71 42 pf-v6-u-(margin_padding)-0

Omit 0 value because fewer occurrences and in case it is more needed that word values like lg.

Solution

Use typescript-eslint playground to understand string values of className prop.

https://typescript-eslint.io/play/#ts=5.9.3&showAST=es&fileType=.tsx

  1. Edit pluginLimited.js file.

    • Add no-PatternFly-utility-layout lint rule.
  2. Edit eslint.config.js file.

    • Add configuration object with ignores array and comments.

User-facing documentation

  • CHANGELOG.md update is not needed
  • documentation PR is not needed

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added lint rule
  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

  1. npm run tsc in ui/apps/platform folder.

  2. npm run lint:fast-dev in ui/apps/platform folder.

    With configuration object but without ignores property see 413 errors (and get the file paths)

@pedrottimark pedrottimark requested a review from a team as a code owner March 25, 2026 15:59
@rhacs-bot
Copy link
Contributor

Images are ready for the commit at fb51138.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-443-gfb511388f8.

@codecov
Copy link

codecov bot commented Mar 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.28%. Comparing base (2f445e2) to head (fb51138).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #19608   +/-   ##
=======================================
  Coverage   49.28%   49.28%           
=======================================
  Files        2735     2735           
  Lines      206215   206215           
=======================================
  Hits       101630   101630           
  Misses      97043    97043           
  Partials     7542     7542           
Flag Coverage Δ
go-unit-tests 49.28% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants