build: add stylelint for RTL-unsafe logical CSS properties by akashfer · Pull Request #608 · wordpress-mobile/GutenbergKit

akashfer · 2026-08-27T19:48:12Z

Fixes #564

What?

Adds a stylelint setup to GutenbergKit's SCSS, using stylelint-plugin-logical-css to flag physical (left/right/padding-left/etc.) properties that should be written as logical properties (inset-inline-start, padding-inline, etc.) for RTL correctness. Converts the ~29 existing violations the new rule flags to their logical equivalents.

Why?

GutenbergKit's own SCSS is never processed by rtlcss, and nothing currently guards against physical-property RTL bugs — one (border-right-color misplacing the toolbar divider) had to be found by eye during RTL testing. There was no stylelint setup at all: no config, no dependency, no lint:css script, no Makefile target.

How?

Added stylelint, stylelint-plugin-logical-css (pinned to ^1.2.3, matching the version upstream Gutenberg pins in tools/stylelint/config.js — v2 renamed the rule this issue asks for), @wordpress/stylelint-config, and postcss-scss as dev dependencies.
Added .stylelintrc.mjs extending @wordpress/stylelint-config/scss and enabling plugin/use-logical-properties-and-values, reusing upstream's ignore list for properties that don't affect RTL (margin-top, width, overflow-y, border-top, etc.).
The base @wordpress/stylelint-config/scss ruleset is otherwise stricter than this codebase's existing conventions (BEM-style double-underscore class names, blank-line formatting, specificity ordering). Relaxed the same rules upstream's own config relaxes for the same reason, to keep this PR scoped to the logical-properties rule rather than a repo-wide style rewrite.
Added lint:css / lint:css:fix npm scripts and matching lint-css / lint-css-fix Makefile targets, mirroring the existing lint-js pattern.
Wired lint-css into the Buildkite pipeline alongside lint-js, including the release-gate depends_on list.
Converted the ~29 flagged declarations (all symmetric left/right pairs, or single vertical properties like scroll-padding-bottom) to their logical equivalents.
Two declarations in the toolbar's scroll-indicator gradients (::before/::after) are not a symmetric pair — they're the left- and right-scroll affordances, and whether they should flip in RTL needs its own investigation. Left those physical with an inline stylelint-disable-next-line and a comment, rather than guessing at a fix.
Along the way, fixed a handful of small pre-existing issues the new linter surfaced independent of RTL (a dead duplicate position: absolute overridden by a later position: fixed !important, two duplicate selector blocks that can merge into one, three 0px → 0 unit cleanups, and a few non-shorthand/named colors).

Testing Instructions

npm run lint:css (or make lint-css) — should pass with no errors.
make lint-js / make test-js / make build — included here to confirm the change doesn't affect JS lint, unit tests (227 passing), or the Vite build.
Spot-check the converted rules still render correctly, e.g. the fixed toolbar (src/components/editor-toolbar/style.scss) and the visual editor toolbar/error boundary (src/components/visual-editor/style.scss) in both LTR and RTL (?lang=ar or similar).

Accessibility Testing Instructions

No UI/behavior changes — this is a build-tooling and CSS-property-name change only; visual output is unchanged (logical properties resolve to the same physical box-model values in LTR, which is this project's current default and only tested direction).

GutenbergKit's SCSS was never linted for physical left/right properties, so a direction-specific bug (border-right-color on the toolbar) had to be found by eye. Add stylelint with stylelint-plugin-logical-css, mirroring the setup and ignore list upstream Gutenberg uses in tools/stylelint/config.js, and wire lint:css / lint-css into npm scripts, the Makefile, and CI alongside lint:js / lint-js. Also converts the ~29 existing physical-property declarations the new rule flags to their logical equivalents. Two declarations in the toolbar's scroll-indicator gradients are not a symmetric pair and are left physical with a stylelint-disable, pending separate investigation into their RTL behavior. The base @wordpress/stylelint-config/scss ruleset is otherwise stricter than this codebase's existing conventions; the selector-naming, blank-line, and specificity-ordering rules it would also enable are relaxed the same way upstream's own config relaxes them, to keep this change scoped to the logical-properties rule.

AliSoftware · 2026-08-28T09:39:23Z

In terms of infra/tooling (Makefile change & package.json changes) the changes looks ok to me.
But in terms of the CSS changes, I'm adding @dcalhoun as reviewer because he'll probably be more knowledgable in the changes this PR makes to the CSS properties in the GutenbergKit's code itself than our team would.

dcalhoun · 2026-08-28T13:05:33Z

@akashfer thank you for exploring this. It may take me a little while to begin review of this, but hopefully I can next week. I'll follow up.

akashfer · 2026-08-28T13:06:50Z

Okay thanks

dcalhoun

@akashfer thank you for contributing this! 🙇🏻‍♂️

I tested the editor with the style changes. I did not encounter any regressions.

In my review with Claude, I did uncover several findings that I believe are legitimate and worth addressing. Would you please take a look at addressing the inline comments?

Lastly, if you merge the latest trunk branch into this branch, we'll receive #615, which will help avoid cryptic lint run hangs.

dcalhoun · 2026-09-16T18:49:26Z

+// parent editor toolbar to avoid nested scrolling views. Also disable scrolling
+// of the block toolbar itself, relying on the parent container scrolling instead.


It seems we can forgo the copy of this comment from line 119. It duplicates the existing comment.

Suggested change

// parent editor toolbar to avoid nested scrolling views. Also disable scrolling

// of the block toolbar itself, relying on the parent container scrolling instead.

// parent editor toolbar to avoid nested scrolling views.

dcalhoun · 2026-09-16T19:42:16Z

+/** @type {import('stylelint').Config} */
+export default {
+	extends: '@wordpress/stylelint-config/scss',
+	plugins: ['stylelint-plugin-logical-css'],


Finding from Claude:

This file fails prettier --check (wp-prettier's bracket spacing). CI won't catch it: formatting is enforced via the prettier/prettier ESLint rule, and lint:js runs eslint . --ext js,jsx, so .mjs is never linted.

Suggested change

plugins: ['stylelint-plugin-logical-css'],

plugins: [ 'stylelint-plugin-logical-css' ],

Worth adding mjs,cjs to the --ext list in a follow-up — .eslintrc.cjs, .prettierrc.cjs, and e2e/.eslintrc.cjs all already pass, so it'd be a no-op today.

dcalhoun · 2026-09-16T19:42:17Z

+export default {
+	extends: '@wordpress/stylelint-config/scss',
+	plugins: ['stylelint-plugin-logical-css'],
+	rules: {


Finding from Claude:

Gutenberg's tools/stylelint/config.js sets both of these; without them the stylelint-disable-next-line comments added below can rot silently, and a stale disable will suppress whatever lands on the line after it. lint:js already enforces the equivalent via --report-unused-disable-directives.

Suggested change

rules: {

reportNeedlessDisables: true,

reportDescriptionlessDisables: true,

rules: {

reportDescriptionlessDisables requires a -- reason on each disable directive, which pairs with the toolbar comments.

dcalhoun · 2026-09-16T19:42:18Z

+	extends: '@wordpress/stylelint-config/scss',
+	plugins: ['stylelint-plugin-logical-css'],
+	rules: {
+		'plugin/use-logical-properties-and-values': [


Finding from Claude:

Gutenberg is on stylelint-plugin-logical-css@^2.1.0, where this rule was renamed and split in two. Worth moving now rather than landing disable comments under a name we'd have to rename later — ^1.2.3 can't cross the major, so the pin freezes here otherwise.

I ran v2 against src/**/*.scss with this exact ignore list: the only violations are the two toolbar lines that already carry disables, flagged because v2 doesn't recognise the v1 name in them. Nothing else in the codebase changes.

Suggested change

'plugin/use-logical-properties-and-values': [

'logical-css/require-logical-keywords': true,

'logical-css/require-logical-properties': [

Goes together with the version bump on package.json and the two disable directives in editor-toolbar/style.scss.

dcalhoun · 2026-09-16T19:42:19Z

+					// Doesn't affect RTL styles
+					'border-bottom',
+					'border-top',
+					'width',
+					'min-width',
+					'max-width',
+					'height',
+					'min-height',
+					'max-height',
+					'margin-top',
+					'margin-bottom',
+					'overflow-x',
+					'overflow-y',
+					'padding-top',
+					'padding-bottom',
+					'scroll-margin-top',
+					'scroll-margin-bottom',
+					'top',
+					'bottom',


Finding from Claude:

The list is copied from Gutenberg and inherits its gaps: it ignores the block-axis shorthands but not the longhands. Per the plugin's physical.js, these are also flagged and equally unable to flip in RTL:

Suggested change

// Doesn't affect RTL styles

'border-bottom',

'border-top',

'width',

'min-width',

'max-width',

'height',

'min-height',

'max-height',

'margin-top',

'margin-bottom',

'overflow-x',

'overflow-y',

'padding-top',

'padding-bottom',

'scroll-margin-top',

'scroll-margin-bottom',

'top',

'bottom',

// Doesn't affect RTL styles

'border-bottom',

'border-bottom-color',

'border-bottom-style',

'border-bottom-width',

'border-top',

'border-top-color',

'border-top-style',

'border-top-width',

'box-orient',

'contain-intrinsic-height',

'contain-intrinsic-width',

'width',

'min-width',

'max-width',

'height',

'min-height',

'max-height',

'margin-top',

'margin-bottom',

'overflow-x',

'overflow-y',

'overscroll-behavior-x',

'overscroll-behavior-y',

'padding-top',

'padding-bottom',

'scroll-margin-top',

'scroll-margin-bottom',

'scroll-padding-top',

'scroll-padding-bottom',

'top',

'bottom',

(border-top-left-radius and friends stay flagged — they name a side on the inline axis.) This is what forced the scroll-padding-bottom conversion in src/index.scss.

dcalhoun · 2026-09-16T19:42:25Z

+		// Not a symmetric pair with the `right` below (see the `::after` gradient) —
+		// whether this should flip in RTL needs its own investigation, so it is
+		// intentionally left physical for now rather than converted here.
+		// stylelint-disable-next-line plugin/use-logical-properties-and-values


Finding from Claude:

The stated reason doesn't hold — this isn't pending investigation, it's a permanent constraint. left: 0 pairs with the hard-coded linear-gradient(to right) three lines below; linear-gradient has no logical direction, so converting the position alone would put the fade on the wrong edge in RTL. Both have to flip together or neither does.

Suggested change

// Not a symmetric pair with the `right` below (see the `::after` gradient) —

// whether this should flip in RTL needs its own investigation, so it is

// intentionally left physical for now rather than converted here.

// stylelint-disable-next-line plugin/use-logical-properties-and-values

// stylelint-disable-next-line plugin/use-logical-properties-and-values -- paired with the gradient direction below

The directive will need renaming to logical-css/require-logical-properties if the plugin is bumped to v2.

dcalhoun · 2026-09-16T19:42:26Z

+		// Not a symmetric pair with the `left` above — see the note on the
+		// `::before` gradient.
+		// stylelint-disable-next-line plugin/use-logical-properties-and-values


Finding from Claude:

Same as above — right: 0 pairs with linear-gradient(to left) below, which is the real reason this can't be converted in isolation.

Suggested change

// Not a symmetric pair with the `left` above — see the note on the

// `::before` gradient.

// stylelint-disable-next-line plugin/use-logical-properties-and-values

// stylelint-disable-next-line plugin/use-logical-properties-and-values -- paired with the gradient direction below

The directive will need renaming to logical-css/require-logical-properties if the plugin is bumped to v2.

dcalhoun · 2026-09-16T20:12:44Z

 		"patch-package": "^8.0.1",
+		"postcss-scss": "^4.0.9",


Finding from Claude:

postcss-scss is a direct dependency of stylelint-config-recommended-scss, which requires it internally to set customSyntax. Nothing in this repo imports it, so declaring it here is dead weight we'd have to version-bump by hand.

Suggested change

"patch-package": "^8.0.1",

"postcss-scss": "^4.0.9",

"patch-package": "^8.0.1",

dcalhoun · 2026-09-16T20:12:45Z

 		"react-devtools-core": "^7.0.1",
 		"sass-embedded": "^1.98.0",
+		"stylelint": "^16.26.1",
+		"stylelint-plugin-logical-css": "^1.2.3",


Finding from Claude:

Bump to match Gutenberg — see the rule-name comment on .stylelintrc.mjs, which has the measured cost. Apply both together or neither; the renamed rules won't resolve on v1.

Suggested change

"stylelint-plugin-logical-css": "^1.2.3",

"stylelint-plugin-logical-css": "^2.1.0",

dcalhoun · 2026-09-16T20:12:46Z

 		"sass-embedded": "^1.98.0",
+		"stylelint": "^16.26.1",
+		"stylelint-plugin-logical-css": "^1.2.3",
 		"vite": "^8.0.16",


Finding from Claude:

stylelint-scss is missing. @wordpress/stylelint-config declares it as a peer and its scss.js loads it by name (plugins: ['stylelint-scss']), so it only resolves today because npm auto-installs and hoists the peer. Under a non-hoisting installer or --legacy-peer-deps, config load fails outright.

Suggested change

"vite": "^8.0.16",

"stylelint-scss": "^6.4.0",

"vite": "^8.0.16",

akashfer requested a review from a team as a code owner August 27, 2026 19:48

github-actions Bot added the [Type] Build Tooling Issues or PRs related to build tooling label Aug 27, 2026

AliSoftware requested a review from dcalhoun August 28, 2026 09:37

dcalhoun requested changes Sep 16, 2026

View reviewed changes

		// parent editor toolbar to avoid nested scrolling views. Also disable scrolling
		// of the block toolbar itself, relying on the parent container scrolling instead.

	plugins: ['stylelint-plugin-logical-css'],
	plugins: [ 'stylelint-plugin-logical-css' ],

	'plugin/use-logical-properties-and-values': [
	'logical-css/require-logical-keywords': true,
	'logical-css/require-logical-properties': [

	"patch-package": "^8.0.1",
	"postcss-scss": "^4.0.9",
	"patch-package": "^8.0.1",

	"stylelint-plugin-logical-css": "^1.2.3",
	"stylelint-plugin-logical-css": "^2.1.0",

	"vite": "^8.0.16",
	"stylelint-scss": "^6.4.0",
	"vite": "^8.0.16",

Conversation

akashfer commented Aug 27, 2026

What?

Why?

How?

Testing Instructions

Accessibility Testing Instructions

Uh oh!

AliSoftware commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcalhoun commented Aug 28, 2026

Uh oh!

akashfer commented Aug 28, 2026

Uh oh!

dcalhoun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AliSoftware commented Aug 28, 2026 •

edited

Loading