Skip to content

chore(css-modules): 🤖 Enable linter on CSS files#945

Merged
punkbit merged 22 commits into
mainfrom
chore/enable-linter-on-css-files
Apr 10, 2026
Merged

chore(css-modules): 🤖 Enable linter on CSS files#945
punkbit merged 22 commits into
mainfrom
chore/enable-linter-on-css-files

Conversation

@punkbit
Copy link
Copy Markdown
Contributor

@punkbit punkbit commented Mar 24, 2026

Why?

Previously, whether using Styled-Components or our new CSS Modules, there were no safeguards to prevent the use of CSS features unsupported by major browsers. As part of our migration to CSS Modules, this PR introduces CSS linting to the project. This ensures all new styles adhere to browser compatibility standards and maintain consistent linting rules.

During the transition from Styled-Components, the process of porting styles can lead to confusion between migration related adjustments and actual browser support issues. Introducing the linter now provides clarity, ensuring that any compatibility errors are caught automatically rather than being mistaken for migration/porting errors/issues.

⚠️ Depends on #810 (should be merged first)

How?

  • Uses stylelint-config-standard as base
  • Introduces browserlist and filters out obscure browsers while supporting modern ones (Chrome 100+,
    Safari 15.5+, Firefox 99+)
  • stylelint-no-unsupported-browser-features for browser compatibility, e.g. errors on unsupported features
  • Excludes auto-generated token files
  • Updates package.json scripts, e.g. clean separation of code (ESLint) and style (Stylelint) linting
  • Add property order css linter rule
  • Exclude obscure browser support from the list
  • Update documentation

Preview?

N/A

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Mar 24, 2026

⚠️ No Changeset found

Latest commit: 26b2052

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@punkbit punkbit changed the title chore(linting): 🤖 enable linter on css files chore(linting): 🤖 Enable linter on CSS files Mar 24, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 24, 2026

Claude finished @punkbit's task in 1m 23s —— View job


Code Review

Solid tooling addition — the config is well-structured, the BEM pattern is correct, the justification comments in stylelint.config.js (css-nesting transpiled, :focus-visible fallback, etc.) are excellent for maintainability. A couple of real issues to address:


🚨 Must Fix

  • .browserslistrc lines 2–3 — floating rolling window: last 4 years and > 0.5% are separate OR-unioned queries, creating a non-deterministic, ever-changing browser set. last 4 years today reaches back to 2022 browsers; a year from now it shifts forward, potentially breaking CSS rules that pass today. The PR description explicitly states the intent is Chrome 100+, Safari 15.5+, Firefox 99+ — but those floors aren't actually enforced here. Consider replacing with pinned minimums to match the stated intent:
    chrome >= 100
    safari >= 15.5
    firefox >= 99
    not and_qq > 0
    not baidu > 0
    not and_uc > 0
    not kaios > 0
    not op_mini all
    
    Fix this →

♿ Accessibility

  • Button.module.css lines 254–258 — prefers-reduced-motion only covers animation, not transitions: The @media (prefers-reduced-motion: reduce) block disables the shimmer animation but leaves transition: var(--click-transition-default) active on all four :hover:not(:disabled) states (lines 49, 86, 123, 160). Users with motion sensitivity will still experience hover transitions. Consider adding transition: none for all hover selectors inside that block. Fix this →

💡 Suggestions (non-blocking)

  • Button.module.css lines 49, 86, 123, 160 — transition on :hover instead of base: Setting transition inside :hover selectors means the state change animates in but snaps back instantly on mouse-out. Standard practice places transition on the base .button rule so both directions animate smoothly. Unless the snap-back is intentional, this is worth moving to the base rule.

  • Button.module.css line 14 (and hover/active/disabled states) — font shorthand: font: var(...) resets all sub-properties (style, variant, weight, stretch, size, line-height, family) to the token value or browser defaults. Verify the token fully specifies all relevant sub-properties; otherwise states that only change e.g. font-weight could silently reset line-height or font-family.


Great work overall — the tooling infrastructure is solid and the PR description is thorough. 🙌

@ClickHouse ClickHouse deleted a comment from github-actions Bot Mar 24, 2026
@ClickHouse ClickHouse deleted a comment from github-actions Bot Mar 24, 2026
@punkbit punkbit requested a review from hoorayimhelping March 24, 2026 15:41
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 24, 2026

Claude finished @punkbit's task in 1m 29s —— View job


Code Review

Solid tooling setup — the stylelint.config.js is well-organized, the inline // NOTE: justifications for ignored browser features are great for maintainability, and the BEM selector pattern is correct. A few issues from the previous review are still open, plus one new regression introduced by this push:


🚨 Must Fix

  • .browserslistrc lines 2–3 — floating rolling window (unresolved from previous review): last 4 years and > 0.5% are OR-unioned and non-deterministic. Today "last 4 years" reaches back to 2022; next year it shifts, silently widening the supported set and potentially passing CSS that fails on browsers you didn't intend to support. The PR description states the intent is Chrome 100+, Safari 15.5+, Firefox 99+ — but those floors aren't enforced. Replace with pinned minimums:

    chrome >= 100
    safari >= 15.5
    firefox >= 99
    not and_qq > 0
    not baidu > 0
    not and_uc > 0
    not kaios > 0
    not op_mini all
    

    Fix this →

  • Button.module.csstransition moved to :hover, removing the base transition (regression): The previous code had transition: font var(--click-transition-default) on the base .button rule. This push moved transition: var(--click-transition-default) into each :hover:not(:disabled) selector (lines 49, 86, 123, 160). This creates asymmetric animation: hover-in animates, hover-out snaps back instantly. Standard practice is to place transition on the base rule so both directions animate. Unless the snap-back is intentional, this should be moved back to .button {}.
    Fix this →


♿ Accessibility

  • Button.module.css lines 254–258 — prefers-reduced-motion only covers animation (unresolved from previous review): The @media (prefers-reduced-motion: reduce) block disables the shimmer animation but the transition on all four :hover:not(:disabled) selectors remains active. Users with motion sensitivity will still see hover transitions. Add transition: none inside the prefers-reduced-motion block for all hover selectors.
    Fix this →

💡 Suggestions (non-blocking)

  • Button.module.cssfont shorthand resets all sub-properties: font: var(...) on :hover, :active, :disabled states resets every font sub-property (style, variant, weight, stretch, size, line-height, family) to whatever the token provides or browser defaults. If the token doesn't fully specify all sub-properties, you could silently lose line-height or font-family in certain states. Worth verifying the token covers all sub-properties, or consider switching to just font-weight (or whichever property actually changes per state).

Comment thread stylelint.config.js
Comment thread stylelint.config.js
Comment thread stylelint.config.js
Comment thread README.md Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 24, 2026

Claude finished @punkbit's task in 1m 7s —— View job


Code Review

The tooling setup is clean and well-structured — the inline // NOTE: justifications in stylelint.config.js are great for long-term maintainability. Three issues from previous reviews remain unresolved, flagged below.


🚨 Must Fix

  • .browserslistrc lines 2–3 — floating rolling window (flagged twice before, still unresolved): last 4 years and > 0.5% are OR-unioned and non-deterministic. The README table documents Chrome 100+, Edge 100+, Firefox 99+, Safari 15.5+ as minimums, but the actual config doesn't enforce those floors — last 4 years today reaches back to 2022 and will silently shift forward over time. Replace with pinned minimums to match the documented intent:

    chrome >= 100
    edge >= 100
    safari >= 15.5
    firefox >= 99
    not and_qq > 0
    not baidu > 0
    not and_uc > 0
    not kaios > 0
    not op_mini all
    

    Fix this →

  • Button.module.css lines 49, 86, 123, 160 — transition on :hover creates asymmetric animation (regression, flagged in previous review): transition is set inside each :hover:not(:disabled) rule, so hover-in animates smoothly but hover-out snaps back instantly. Move transition: var(--click-transition-default) to the base .button {} rule for symmetric animation in both directions.
    Fix this →


♿ Accessibility

  • Button.module.css lines 254–258 — prefers-reduced-motion doesn't suppress transition (flagged in previous review): The @media (prefers-reduced-motion: reduce) block only disables the shimmer animation. The transition on all four :hover:not(:disabled) selectors remains active, so users with motion sensitivity still get hover transitions. Once transition is moved to .button {}, add transition: none inside the reduced-motion block to cover it.
    Fix this →

💡 Suggestions (non-blocking)

  • stylelint.config.js line 114 — BEM selector pattern: DreaminDani flagged this as a potential blocker if the BEM RFC isn't approved first. Punkbit agreed to remove it if needed. Worth a quick decision before merge to avoid locking in a convention that might be revised.

  • Button.module.cssfont shorthand on state selectors: font: var(...) on :hover, :active, :disabled, and :focus-visible resets every font sub-property (weight, style, size, line-height, family) to whatever the token resolves to. If any token only specifies a subset of sub-properties, you may silently lose line-height or font-family in those states. Worth verifying the tokens fully specify all sub-properties, or switching to font-weight / the specific property that actually changes.


Comment thread stylelint.config.js Outdated
Comment thread stylelint.config.js
Comment thread src/components/Button/Button.module.css
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 27, 2026

Claude finished @punkbit's task in 1m 44s —— View job


Code Review

The stylelint infrastructure is well-structured — the inline // NOTE: justifications, the property ordering, and the overall config design are solid. Three issues from previous reviews remain unresolved.


🚨 Must Fix

  • .browserslistrc lines 2–3 — floating rolling window (flagged three times, still unresolved): last 4 years and > 0.5% are OR-unioned and non-deterministic. "Last 4 years" currently reaches back to 2022 and shifts forward each year, silently changing the supported browser set without any code change. The README documents Chrome 100+, Edge 100+, Firefox 99+, Safari 15.5+ as the minimums — but those floors aren't actually enforced by this config. Replace with pinned minimums:

    chrome >= 100
    edge >= 100
    safari >= 15.5
    firefox >= 99
    not and_qq > 0
    not baidu > 0
    not and_uc > 0
    not kaios > 0
    not op_mini all
    

    Fix this →

  • Button.module.css lines 49, 86, 123, 160 — transition on :hover creates asymmetric animation (unresolved): transition is set inside each :hover:not(:disabled) rule, so hover-in animates but hover-out snaps back instantly. Move transition: var(--click-transition-default) to the base .button {} rule for symmetric animation in both directions.
    Fix this →


♿ Accessibility

  • Button.module.css lines 254–258 — prefers-reduced-motion doesn't suppress transition (unresolved): The @media (prefers-reduced-motion: reduce) block only disables the shimmer animation. Once transition is moved to the base .button {} rule (above), add transition: none inside this block to ensure users with motion sensitivity don't see hover transitions.
    Fix this →

💡 Suggestions (non-blocking)

  • stylelint.config.js line 115 — stale comment: The comment references #two-dashes-style and the message says mod-name_mod-val, but the actual class names now use classic BEM (single underscore: button_primary, button_loading). The link and description no longer match the convention in use — worth updating the comment to avoid confusion for future contributors.

Comment thread stylelint.config.js Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 27, 2026

Claude finished @punkbit's task in 1m 33s —— View job


Code Review

Good progress — the stale #two-dashes-style anchor is now resolved (✅). The stylelint.config.js comment and URL now correctly describe classic BEM. Three issues from previous reviews remain open.


🚨 Must Fix

  • .browserslistrc lines 2–3 — floating rolling window (flagged four times, still unresolved): last 4 years and > 0.5% are OR-unioned and non-deterministic. "Last 4 years" today reaches back to 2022 and silently shifts forward each year, changing the effective browser set without any code change. The README documents Chrome 100+, Edge 100+, Firefox 99+, Safari 15.5+ as minimums — these floors aren't enforced. Replace with pinned minimums:

    chrome >= 100
    edge >= 100
    safari >= 15.5
    firefox >= 99
    not and_qq > 0
    not baidu > 0
    not and_uc > 0
    not kaios > 0
    not op_mini all
    

    Fix this →

  • Button.module.css lines 49, 86, 123, 160 — transition on :hover creates asymmetric animation (unresolved): transition: var(--click-transition-default) is inside each :hover:not(:disabled) rule — hover-in animates smoothly, hover-out snaps back instantly. Move it to the base .button {} rule for symmetric animation in both directions.
    Fix this →


♿ Accessibility

  • Button.module.css lines 254–258 — prefers-reduced-motion doesn't suppress transition (unresolved): The reduced-motion block only disables the shimmer animation. Once transition is moved to .button {} (above), add transition: none inside this block so users with motion sensitivity don't see hover transitions either.
    Fix this →

@punkbit punkbit changed the title chore(linting): 🤖 Enable linter on CSS files chore(css-modules): 🤖 Enable linter on CSS files Apr 1, 2026
@punkbit punkbit force-pushed the chore/enable-linter-on-css-files branch from 9811a27 to 24d96c7 Compare April 7, 2026 14:34
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

Claude finished @punkbit's task in 54s —— View job


Code Review

Good progress on the BEM naming convention and comment/link updates (both now resolved ✅). Three issues from previous reviews are still open.


🚨 Must Fix

  • .browserslistrc lines 2–3 — floating rolling window (flagged five times, still unresolved): last 4 years and > 0.5% are OR-unioned and non-deterministic. "Last 4 years" today reaches back to 2022 and silently shifts forward each year, changing the effective browser set without any code change. The README documents Chrome 100+, Edge 100+, Firefox 99+, Safari 15.5+ as minimums — these floors aren't enforced. Replace with pinned minimums:

    chrome >= 100
    edge >= 100
    safari >= 15.5
    firefox >= 99
    not and_qq > 0
    not baidu > 0
    not and_uc > 0
    not kaios > 0
    not op_mini all
    

    Fix this →

  • Button.module.css lines 49, 86, 123, 160 — transition on :hover creates asymmetric animation (unresolved): transition: var(--click-transition-default) is inside each :hover:not(:disabled) rule — hover-in animates smoothly, hover-out snaps back instantly. Move it to the base .button {} rule for symmetric animation in both directions.
    Fix this →


♿ Accessibility

  • Button.module.css lines 257–261 — prefers-reduced-motion doesn't suppress transition (unresolved): The @media (prefers-reduced-motion: reduce) block only disables the shimmer animation. The transition on all four :hover:not(:disabled) selectors remains active. Once transition is moved to .button {} (above), add transition: none inside this block so users with motion sensitivity don't see hover transitions.
    Fix this →

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

Claude finished @punkbit's task in 45s —— View job


Code Review

The stylelint infrastructure is well-structured — property ordering, the inline // NOTE: justifications in stylelint.config.js, and BEM naming are all solid. Three issues from previous reviews remain unresolved.


🚨 Must Fix

  • .browserslistrc lines 2–3 — floating rolling window (flagged five times, still unresolved): last 4 years and > 0.5% are OR-unioned and non-deterministic. "Last 4 years" today reaches back to 2022 and silently shifts forward each year, changing the effective browser set without any code change. The README documents Chrome 100+, Edge 100+, Firefox 99+, Safari 15.5+ as minimums — these floors aren't enforced. Replace with pinned minimums:

    chrome >= 100
    edge >= 100
    safari >= 15.5
    firefox >= 99
    not and_qq > 0
    not baidu > 0
    not and_uc > 0
    not kaios > 0
    not op_mini all
    

    Fix this →

  • Button.module.css lines 49, 86, 123, 160 — transition on :hover creates asymmetric animation (unresolved): transition: var(--click-transition-default) is inside each :hover:not(:disabled) rule — hover-in animates smoothly, hover-out snaps back instantly. Move it to the base .button {} rule for symmetric animation in both directions.
    Fix this →


♿ Accessibility

  • Button.module.css lines 257–261 — prefers-reduced-motion doesn't suppress transition (unresolved): The @media (prefers-reduced-motion: reduce) block only disables the shimmer animation. The transition on all four :hover:not(:disabled) selectors remains active. Once transition is moved to .button {} (above), add transition: none inside this block so users with motion sensitivity don't see hover transitions.
    Fix this →

@punkbit punkbit force-pushed the chore/init-css-modules-setup branch from 21669c4 to dc3dbf8 Compare April 10, 2026 12:10
@punkbit punkbit force-pushed the chore/enable-linter-on-css-files branch from 21b0c94 to 2100e8b Compare April 10, 2026 12:16
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 10, 2026

Claude finished @punkbit's task in 1m 10s —— View job


Code Review

The BEM renaming and comment/link updates from previous reviews are resolved ✅. Three issues from prior reviews remain open on the current code.


🚨 Must Fix

  • .browserslistrc lines 2–3 — floating rolling window (flagged six times, still unresolved): last 4 years and > 0.5% are OR-unioned and non-deterministic. "Last 4 years" today reaches back to 2022 and silently shifts forward each year, changing the effective browser set without any code change. The README documents Chrome 100+, Edge 100+, Firefox 99+, Safari 15.5+ as minimums — these floors aren't enforced. Replace with pinned minimums:

    chrome >= 100
    edge >= 100
    safari >= 15.5
    firefox >= 99
    not and_qq > 0
    not baidu > 0
    not and_uc > 0
    not kaios > 0
    not op_mini all
    

    Fix this →

  • Button.module.css lines 49, 86, 123, 160 — transition on :hover creates asymmetric animation (unresolved): transition: var(--click-transition-default) is inside each :hover:not(:disabled) rule — hover-in animates smoothly, hover-out snaps back instantly. Move it to the base .button {} rule for symmetric animation in both directions.
    Fix this →


♿ Accessibility

  • Button.module.css lines 257–261 — prefers-reduced-motion doesn't suppress transition (unresolved): The @media (prefers-reduced-motion: reduce) block only disables the shimmer animation. The transition on all four :hover:not(:disabled) selectors remains active, so users with motion sensitivity still experience hover transitions. Once transition is moved to .button {} (above), add transition: none inside this block to cover it.
    Fix this →

Base automatically changed from chore/init-css-modules-setup to main April 10, 2026 13:17
@punkbit punkbit force-pushed the chore/enable-linter-on-css-files branch from 2100e8b to 26b2052 Compare April 10, 2026 13:28
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 10, 2026

Claude finished @punkbit's task in 1m 4s —— View job


Code Review

The stylelint infrastructure is well-structured overall — the inline // NOTE: justifications in stylelint.config.js, the property ordering, and the BEM naming pattern are all solid. Three issues from prior reviews remain unresolved on the current code.


🚨 Must Fix

  • .browserslistrc lines 2–3 — floating rolling window (flagged six times, still unresolved): last 4 years and > 0.5% are OR-unioned and non-deterministic. "Last 4 years" today reaches back to 2022 and silently shifts forward each year, changing the effective browser set without any code change. The README documents Chrome 100+, Edge 100+, Firefox 99+, Safari 15.5+ as minimums — these floors aren't enforced by the actual config. Replace with pinned minimums to match the documented intent:

    chrome >= 100
    edge >= 100
    safari >= 15.5
    firefox >= 99
    not and_qq > 0
    not baidu > 0
    not and_uc > 0
    not kaios > 0
    not op_mini all
    

    Fix this →

  • Button.module.css lines 49, 86, 123, 160 — transition on :hover creates asymmetric animation (unresolved): transition: var(--click-transition-default) is inside each :hover:not(:disabled) rule — hover-in animates smoothly, hover-out snaps back instantly. Move it to the base .button {} rule for symmetric animation in both directions.
    Fix this →


♿ Accessibility

  • Button.module.css lines 257–261 — prefers-reduced-motion doesn't suppress transition (unresolved): The @media (prefers-reduced-motion: reduce) block only disables the shimmer animation. The transition on all four :hover:not(:disabled) selectors remains active, so users with motion sensitivity still experience hover transitions. Once transition is moved to .button {} (above), add transition: none inside this block to cover it.
    Fix this →

@workflow-authentication-public
Copy link
Copy Markdown
Contributor

📚 Storybook Preview Deployed

✅ Preview URL: https://click-6gp8jy41k-clickhouse.vercel.app

Built from commit: 0ab89c07f51e8aa564e806a50a973158f78cc026

@punkbit punkbit merged commit 0347b89 into main Apr 10, 2026
8 checks passed
@punkbit punkbit deleted the chore/enable-linter-on-css-files branch April 10, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants