Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/js_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ jobs:
run: npm ci --no-audit
- name: Run linter
run: npm run lint
- name: Run stylelint
run: npm run stylelint
- name: Run tests
run: npm run test
- name: Publish Coveralls
Expand Down
43 changes: 43 additions & 0 deletions .github/workflows/stylelint_rules_tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
name: Stylelint Rules Tests

on:
pull_request:
paths:
- 'script/lint/**'

permissions:
contents: read

concurrency:
group: ${{ github.ref_name }}-${{ github.workflow }}
cancel-in-progress: true

jobs:
setup_matrix:
name: Setup matrix
runs-on: ubuntu-latest
outputs:
matrix: ${{ steps.build_matrix.outputs.matrix }}
steps:
- name: Build test matrix
id: build_matrix
uses: theforeman/gha-matrix-builder@v0

test:
runs-on: ubuntu-latest
needs: setup_matrix
strategy:
fail-fast: false
matrix: ${{ fromJson(needs.setup_matrix.outputs.matrix) }}
steps:
- uses: actions/checkout@v4
- name: Setup Node ${{ matrix.node }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node }}
- name: Generate package-lock
run: npm install --package-lock-only --no-audit
- name: Install dependencies
run: npm ci --no-audit
- name: Run stylelint rules tests
run: npm run test:stylelint
10 changes: 0 additions & 10 deletions .stylelintrc

This file was deleted.

39 changes: 39 additions & 0 deletions .stylelintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
{
"extends": "stylelint-config-standard-scss",
"ignoreFiles": [
"app/assets/stylesheets/unimported/**"
],
"plugins": [
"./script/lint/stylelint-no-root-pf-overrides.js",
"./script/lint/stylelint-no-bare-element-selectors.js"
],
"rules": {
"no-descending-specificity": null,
"foreman/no-root-pf-overrides": true,
"foreman/no-bare-element-selectors": true,
"selector-class-pattern": null,
"selector-id-pattern": null,
"custom-property-pattern": null,
"scss/dollar-variable-pattern": null,
"at-rule-empty-line-before": null,
"custom-property-empty-line-before": null,
"scss/dollar-variable-empty-line-before": null,
"scss/double-slash-comment-empty-line-before": null,
"scss/double-slash-comment-whitespace-inside": null,
"scss/dollar-variable-colon-space-after": null,
"scss/load-partial-extension": null,
"scss/load-no-partial-leading-underscore": null,
"scss/no-global-function-names": null,
"media-feature-range-notation": null,
"alpha-value-notation": null,
"color-function-notation": null,
"color-function-alias-notation": null,
"font-family-name-quotes": null,
"property-no-vendor-prefix": null,
"shorthand-property-no-redundant-values": null,
"declaration-block-no-redundant-longhand-properties": null,
"declaration-property-value-keyword-no-deprecated": null,
"length-zero-no-unit": null,
"no-invalid-position-at-import-rule": null
}
}
4 changes: 4 additions & 0 deletions app/assets/stylesheets/base-pf4.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// stylelint-disable foreman/no-bare-element-selectors
// stylelint-disable foreman/no-root-pf-overrides

/*
This file exists to make pf3 compatible with pf4.
When migrating from pf3 to pf4, some styles might be removable.
Expand Down Expand Up @@ -56,6 +59,7 @@ a {

.pf-v5-c-alert {
line-height: var(--pf-v5-global--LineHeight--md);

h4 {
margin: 0;
line-height: var(--pf-v5-global--LineHeight--md);
Expand Down
16 changes: 11 additions & 5 deletions app/assets/stylesheets/base.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// stylelint-disable foreman/no-bare-element-selectors
// stylelint-disable foreman/no-root-pf-overrides

html {
height: 100%;
width: 100%;
Expand All @@ -17,12 +20,14 @@ html {
var(--header-height) + var(--banner-height)
);
height: calc(100% - var(--header-height) - var(--banner-height));

.rails-table-toolbar {
padding-bottom: 0;
display: flex;
flex-wrap: wrap;
justify-content: space-between;
}

&.pf-v5-c-page {
display: block;
grid-template-columns: unset;
Expand All @@ -40,6 +45,7 @@ body {
#react-app-root {
height: 100%;
width: 100%;

.pf-v5-c-page {
// pf grid rule stops at @media (min-width: 1200px)
display: grid;
Expand Down Expand Up @@ -597,11 +603,10 @@ table {
overflow-wrap: normal;
padding: 4px;
white-space: pre;
word-wrap: normal;
}

.masked-input {
font-family: 'AllBullets';
font-family: 'AllBullets', sans-serif;
}

.btn-spinner {
Expand Down Expand Up @@ -672,15 +677,16 @@ span.btn a.disabled {
td.pf-v5-c-table__td {
padding-top: 2px;
padding-bottom: 2px;

> button {
padding:0;
}
}
}

.user-banner-present {
// banner height is line height and a small padding
--banner-height: calc(
2 * var(--pf-v5-global--spacer--xs) +
(var(--pf-v5-global--LineHeight--md) * var(--pf-v5-global--FontSize--sm))
); // banner height is line height and a small padding
2 * var(--pf-v5-global--spacer--xs) + (var(--pf-v5-global--LineHeight--md) * var(--pf-v5-global--FontSize--sm))
);
}
4 changes: 3 additions & 1 deletion app/assets/stylesheets/navigation.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
.nav a {
font-size: 13px;
}

.active > a,
.open > a {
background: linear-gradient(
Expand All @@ -18,8 +19,9 @@
}
}

.pf-v5-c-masthead {
#foreman-page .pf-v5-c-masthead {
background: var(--pf-v5-c-masthead--BackgroundColor) image-url('navbar.png');

.user-icon {
margin-right: 10px;
}
Expand Down
16 changes: 5 additions & 11 deletions app/assets/stylesheets/patternfly_and_overrides.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// stylelint-disable foreman/no-bare-element-selectors
// stylelint-disable foreman/no-root-pf-overrides

.control-label {
text-align: left;
padding-left: 0;
Expand Down Expand Up @@ -149,15 +152,6 @@ a {
z-index: 2000;
}

@-moz-document url-prefix() {
/* Workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=688556 */
*,
*::before,
*::after {
background-clip: padding-box;
}
}

.smart-var-left {
max-height: 500px;
direction: rtl;
Expand Down Expand Up @@ -194,7 +188,7 @@ a {
}

.tooltip {
word-wrap: break-word;
overflow-wrap: break-word;
}

.has-error {
Expand Down Expand Up @@ -350,7 +344,7 @@ span.btn-action.btn-primary a {
ul, ol {
margin-top: 0px;
margin-bottom: 10px;
}*/
} */
margin-bottom: 0;
}

Expand Down
9 changes: 8 additions & 1 deletion app/assets/stylesheets/patternfly_colors_overrides.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,21 @@
.pf-v5-c-page__sidebar {
--pf-v5-c-page__sidebar--Width: 250px;
--pf-v5-c-page__sidebar--BackgroundColor: #{$nav-pf-vertical-bg-color};

#navigation-search .pf-v5-c-text-input-group {
background-color: var(--pf-v5-global--palette--white);

.pf-v5-c-text-input-group__icon{
color: var(--pf-v5-global--palette--black-500);
}

.pf-v5-c-text-input-group__text-input {
color: var(--pf-v5-global--palette--black-1000);

&::placeholder {
color: var(--pf-v5-global--palette--black-500);
}

::placeholder {
color: var(--pf-v5-global--palette--black-500);
}
Expand All @@ -35,10 +40,12 @@
.pf-v5-c-nav__subnav {
--pf-v5-c-nav--c-divider--BackgroundColor: #{$nav-pf-vertical-active-bg-color};
}

.pf-v5-c-nav__item {
--pf-v5-c-nav__item--before--BorderColor: #{$nav-pf-vertical-active-bg-color};
}

.pf-v5-c-notification-badge {
--pf-v5-c-notification-badge--m-read--m-expanded--after--BackgroundColor: var(--pf-v5-global--palette--blue-500)
--pf-v5-c-notification-badge--m-read--m-expanded--after--BackgroundColor: var(--pf-v5-global--palette--blue-500);
}
}
2 changes: 1 addition & 1 deletion developer_docs/pf3-to-pf5-migration-guide.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ PF5 brings its own layout and tokens; old PF3-scoped or component CSS often beco

* *Audit styles* tied to the old component (same directory, imported SCSS/CSS, or global rules keyed on old class names). Open the screen in the app and confirm whether each rule still affects the PF5 markup.
* *Delete* rules that no longer match any DOM, duplicate what PF5 already provides, or only existed to patch PF3 quirks that are gone.
* *Update* rules that still apply (e.g. plugin-specific layout, integration with non-PF surfaces, or genuinely custom visuals). Prefer *specific selectors* so you do not bleed into core or other plugins.
* *Update* rules that still apply (e.g. plugin-specific layout, integration with non-PF surfaces, or genuinely custom visuals). Prefer *specific selectors* so you do not bleed into core or other plugins. Run `npm run stylelint` to lint your CSS/SCSS.
* Only delete CSS if it is component specific and declared in a global file.

## 9. Prefer local React state over Redux when it fits
Expand Down
1 change: 1 addition & 0 deletions developer_docs/plugins.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ The Webpack config is shared with core, so there's no need for custom configurat
Once all the above is set up, then the script `npm run install` executed from root of the core's git checkout installs dependencies for plugins too.
You can run your plugin test by running `npm run test:plugins PLUGIN_NAME` from core.
You can run lint on your plugin by running `npm run lint:plugins PLUGIN_NAME` from core.
You can run Stylelint (CSS/SCSS) on your plugin by running `npm run stylelint:plugins PLUGIN_NAME` from core.

### Entry points

Expand Down
11 changes: 11 additions & 0 deletions developer_docs/ui-testing-guidelines.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,17 @@ npm run lint:plugins <plugin-name>
```
plugin name is optional, if not provided, all plugins will be linted.

=== Running Stylelint (CSS/SCSS)
To lint CSS/SCSS files:
```bash
npm run stylelint
```
To run Stylelint for plugins:
```bash
npm run stylelint:plugins <plugin-name>
```
plugin name is optional, if not provided, all plugins will be checked.

== Writing tests
Capybara integration tests are more thorough -- they run against a real browser with real data from FactoryBot, catching issues that unit tests miss (routing, API integration, component interactions). Use your best judgement, but here are some guidelines:

Expand Down
2 changes: 2 additions & 0 deletions package-exclude.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@
"babel-plugin-dynamic-import-node",
"coveralls",
"cheerio",
"html-tags",
"cross-env",
"identity-obj-proxy",
"jsx-ast-utils",
"jest",
"nock",
"postcss-selector-parser",
"prettier",
"pretty-format",
"react-addons-test-utils",
Expand Down
9 changes: 7 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
"test:current": "tfm-test --watch",
"test:plugins": "./script/npm_test_plugin.js",
"lint:plugins": "./script/npm_lint_plugins.js",
"test:stylelint": "node --test script/lint/stylelint-*.test.js",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be run in CI as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added .github/workflows/stylelint_rules_tests.yml (similar to the other lint pr)

"stylelint": "stylelint 'webpack/**/*.{scss,css}' 'app/**/*.{scss,css}'",
"stylelint:plugins": "./script/npm_stylelint_plugins.js",
"publish-coverage": "tfm-publish-coverage",
"postinstall": "./script/npm_install_plugins.js",
"analyze": "./script/webpack-analyze"
Expand Down Expand Up @@ -143,6 +146,7 @@
"jsx-ast-utils": "^3.3.3",
"mini-css-extract-plugin": "^2.9.1",
"path-browserify": "^1.0.1",
"postcss-selector-parser": "^7.1.1",
"prettier": "^1.19.1",
"pretty-format": "26.6.2",
"react-dnd-test-backend": "^9.4.0",
Expand All @@ -151,8 +155,9 @@
"sass": "~1.60.0",
"sass-loader": "^13.3.2",
"style-loader": "^1.3.0",
"stylelint": "^9.3.0",
"stylelint-config-standard": "^18.0.0",
"html-tags": "^5.1.0",
"stylelint": "^17.9.1",
"stylelint-config-standard-scss": "^17.0.0",
Comment on lines 149 to +160

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theforeman/packaging all new packages are in package-exclude so shouldnt effect packaging

"tabbable": "^6.2.0",
"ts-loader": "^9.5.2",
"typescript": "^5.8.2",
Expand Down
4 changes: 3 additions & 1 deletion script/lint/lint_core_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
'checkbox',
'clearbutton',
'clearfix',
'combinator',
'combobox',
'comms',
'Composable',
Expand Down Expand Up @@ -79,8 +80,8 @@
'i386',
'internets',
'ip6',
'IPv4',

Check warning on line 83 in script/lint/lint_core_config.js

View workflow job for this annotation

GitHub Actions / test (13, 3.3, 22)

You have a misspelled word: Pv4 on String

Check warning on line 83 in script/lint/lint_core_config.js

View workflow job for this annotation

GitHub Actions / test (13, 3.0, 22)

You have a misspelled word: Pv4 on String
'IPv6',

Check warning on line 84 in script/lint/lint_core_config.js

View workflow job for this annotation

GitHub Actions / test (13, 3.3, 22)

You have a misspelled word: Pv6 on String

Check warning on line 84 in script/lint/lint_core_config.js

View workflow job for this annotation

GitHub Actions / test (13, 3.0, 22)

You have a misspelled word: Pv6 on String
'javascript',
'javascripts',
'jed',
Expand Down Expand Up @@ -133,12 +134,12 @@
'popstate',
'posinset',
'posix',
'postcss',
'pqr',
'ptable',
'puppetclass',
'puppetclasses',
'pxe_loader',
'Pv',
'qcow2',
'rbt',
'readonly',
Expand All @@ -163,6 +164,7 @@
'sparc',
'storages',
'stringified',
'stylelint',
'subcomponent',
'subcomponents',
'subnav',
Expand Down
Loading
Loading