Skip to content

css: convert physical LTR/RTL properties to logical ones before csstools/use-logical becomes an error #122

Description

@moodyjmz

TL;DR

@nextcloud/stylelint-config@3.2.2 (arriving with #117) enables csstools/use-logical, which flags physical LTR/RTL properties in css/*.css. It ships at severity: 'warning', so CSS lint stays green — but the upstream config carries an explicit TODO to promote it to an error, at which point the check fails.

CI on main reports 16 warnings across 5 tracked CSS files. Converting them now is cheap; discovering them inside a future @nextcloud/stylelint-config bump is not.

#117 is merged, so the rule is live — numbers below are from the CSS Linting step of run 30554614149 on 99a52bf5, not from a static scan.

Why this has a deadline

From @nextcloud/stylelint-config@3.2.2/index.js:

'csstools/use-logical': [
    'always',
    {
        // TODO: make it an error in the next major or large release
        severity: 'warning',
        // Only lint LTR-RTL properties for now
        except: [
            'top', 'bottom', /-top$/, /-bottom$/, /-top-/, /-bottom-/,
            'width', 'max-width', 'min-width',
            'height', 'max-height', 'min-height',
        ],
    },
],

The except list exempts the block axis (top/bottom) and all sizing properties. It does not exempt left/right, margin-inline/padding-inline physical forms, or left/right values on float/clear/text-align — all three value cases are handled by stylelint-use-logical@2.1.3 (physicalValue map in index.cjs).

So every site below is a live warning on main today, and a live error after the next upstream major.

The 16 warnings, verbatim from CI
File Line Warning
css/editor.css 36:5 Unexpected left property. Use inset-inline-start.
css/main.css 61:5 Unexpected margin-left property. Use margin-inline-start.
css/main.css 126:5 Unexpected margin-right property. Use margin-inline-end.
css/settings.css 54:5 Unexpected left value in float property. Use inline-start.
css/settings.css 58:5 Unexpected margin-left property. Use margin-inline-start.
css/settings.css 66:5 Unexpected margin-left property. Use margin-inline-start.
css/settings.css 88:5 Unexpected left property. Use inset-inline-start.
css/settings.css 92:5 Unexpected margin-left property. Use margin-inline-start.
css/share.css 43:5 Unexpected margin-right property. Use margin-inline-end.
css/share.css 48:5 Unexpected margin-left property. Use margin-inline-start.
css/share.css 53:5 Unexpected margin-left property. Use margin-inline-start.
css/share.css 61:5 Unexpected padding-right property. Use padding-inline.
css/template.css 33:5 Unexpected margin-right property. Use margin-inline-end.
css/template.css 55:5 Unexpected padding-left property. Use padding-inline-start.
css/template.css 63:5 Unexpected left value in float property. Use inline-start.
css/template.css 69:5 Unexpected left property. Use inset-inline-start.

⚠ 16 problems (0 errors, 16 warnings). css/format.css and css/viewer.css are clean.

Note that 16 warnings cover 17 declarations: share.css:61 and :62 are a padding-right/padding-left pair in the same rule (.eurooffice-share-label), and the rule's pair map (['padding-left','padding-right'] → 'padding-inline') reports them once, against line 61, suggesting the padding-inline shorthand. Both lines need replacing by the single shorthand.

This is not a blind find-and-replace

The whole point of the rule is that RTL rendering changes. Two cases need eyes on them rather than a mechanical swap:

  • css/template.css:69left: -10000px is an off-screen visually-hidden hack. Under inset-inline-start it moves off the opposite edge in RTL, which is still off-screen, but it is worth confirming it doesn't introduce horizontal overflow rather than assuming.
  • css/settings.css:88left: 205px pairs with a float: left at line 54 in the same stylesheet; the two need converting together or the layout will disagree with itself in RTL.

margin-left: auto (share.css:53) is a push-to-the-end idiom and becomes margin-inline-start: auto — correct, and it actually fixes RTL rather than merely preserving it.

Worth checking the result with an RTL locale (he, ar, fa) in the editor, share sidebar and settings pages.

Related: local lint results diverge from CI

.gitignore:6 ignores /css/*.chunk.css, but stylelint's only default ignore file is .stylelintignore (stylelint/lib/constants.mjs:6 — it does not read .gitignore), and this repo has no .stylelintignore. @nextcloud/stylelint-config's ignoreFiles covers only **/*.js, **/*.ts, **/*.svg.

CI never notices, because lint-eslint.yml runs npm ci and lints without building. But after a local npm run build, npx stylelint css/*.css also lints the generated Vite chunks — currently 9 extra physical-property hits in css/_plugin-vue_export-helper-*.chunk.css, in code nobody hand-maintains.

Either a .stylelintignore containing css/*.chunk.css, or narrowing the CI glob, would make local and CI runs agree. Cheap, and it removes a source of "why do I get different numbers than CI" confusion while this cleanup is in progress.

Found while reviewing #117 (merged).

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions