BEM Refactor - #75
Merged
Merged
Conversation
Modifiers no longer re-apply shared badge styles; markup must combine .l-ui-badge with a colour/shape modifier, matching strict BEM.
Following audits/bem/2026-05-10:
- Rename `l-ui-page--{vertically-centered,width-constrained}` to
`__vertically-centered`/`__width-constrained` (used as inner elements
of the page block, not modifiers).
- Rename `l-ui-label--checkbox` to `l-ui-checkbox-container__label`
(semantically a child of checkbox-container, not a label variant).
- Rename `l-ui-stream-fade-word` to `l-ui-stream-fade__word`.
- Rename `l-ui-select-wrapper` to `l-ui-select-container` for
consistency with other `*-container` classes.
- Promote `l-ui-table__action` to a real element so `--danger` has a
real base; descendant fallback retained for backwards compatibility.
- Add `l-ui-tabs` block class so `__list`/`__tab`/`__panel` have a parent.
- Replace runtime state classes `.open` and `.hidden` (which collided
with Tailwind's `.hidden` utility) with namespaced BEM modifiers:
`--open` for navigation/backdrop/panel containers and `--hidden`
for the panel button. Stimulus controllers updated to toggle the
new class names.
- Add base classes alongside `l-ui-form__group--large-gap` and
`l-ui-message--sent` in markup so modifiers are paired with their base.
- Document migration in UPGRADING.md.
Following audits/bem/2026-05-10 (Codex), the public surface had four classes that were emitted or documented without matching CSS rules. - Promote `l-ui-checkbox`: replace the descendant selector `.l-ui-checkbox-container input[type="checkbox"]` with an explicit `.l-ui-checkbox` rule. Add the class to bare checkbox markup in devise/sessions and dummy form examples. - Document `l-ui-navigation__section--collapsible` and `l-ui-table__header-cell--sortable` in CSS.md as marker classes (no engine styles; host-app/JS hooks). - Remove `l-ui-text--subtle` from the dummy posts/_title_cell partial and replace with Tailwind utilities; the class had no engine CSS and was used only in this one demo. UPGRADING.md updated.
The `--panel-close` modifier named another block (`panel`) inside the button namespace. The styling (a panel-flush negative margin) is panel-specific positioning, so promote it to a panel element class. Markup composes with the button block: `class="l-ui-button l-ui-button--icon l-ui-panel__close-button"`.
Address reviewer feedback: badge had been simplified to a plain .l-ui-badge class while seven other components (label, page, navigation__item, button, surface, tabs__tab, table-cell-padding) kept the @Utility foo { ... } + .l-ui-foo { @apply foo; } pattern despite being applied in only one place each. The indirection earns its keep only when @Utility is reused; for one consumer it is just an alias. Inline each single-use @Utility into its consumer class. Retain @Utility for entries with real reuse: focus-ring (composition primitive), heading (markdown headings), notice (base + form errors), form__field (field + composer input), and the pagination family (base + Pagy series-nav).
- Add `l-ui-page--with-navigation` to the search-and-replace table (was only mentioned in prose). - Extend the offender-finder grep regex to include `form__group` and `message`, matching the modifiers listed in the migration table. - Move `l-ui-button--panel-close` out of the prepend-base table into the Renamed Classes table only - it's a rename, not a base prepend.
Skill docs and one dummy code sample still referenced the pre-cleanup BEM names or used modifiers without their base: - SKILL.md: rename `--vertically-centered`/`--width-constrained` to the new `__` element form, replace the phantom `l-ui-surface--active` with the real modifiers, and add the `l-ui-tabs` block to the tabs row. - CONTROLLERS.md: add `l-ui-button` base to the modal close button example; add the `l-ui-tabs` block class to the tabs example. - HELPERS.md: add `l-ui-button` base to two example snippets using `l-ui-button--primary`. - dummy tabs.html.erb: the escaped code sample inside the View Code modal still showed `<div data-controller="l-ui--tabs">` without the new `l-ui-tabs` class; now matches the live demo.
Active tab lost all base styling because the controller toggled the base class off when applying --active; now that --active is a true modifier the base class must stay. _field_input also dropped the engine-supplied class whenever a config passed extras[:class], leaving inputs unstyled under strict BEM. Merge extras[:class] with the engine base via token_list so string, array, and conditional-hash class values all flow through, and pass it to hidden fields too.
- Drop the `[class*="l-ui-button--"]:disabled` substring selector; rely on `.l-ui-button:disabled` now that the base class is mandatory. - Class-first action cell: remove the `& button, & a` descendant rule from `.l-ui-table__cell--action`. `l-ui-table__action` is now required on every action element. Updated the helper docstring, dummy docs, and UPGRADING.md note (no longer "retained for backwards compatibility"). - Rename pagination `@utility` shims to neutral hyphenated names (`pagination-base`, `pagination-item`, `pagination-item-active`, `pagination-item-disabled`, `pagination-gap`) so they no longer read as un-namespaced BEM classes; added a header comment explaining they are mixin-only helpers shared with the pagy adapter rules. - Document the `*-container` shell convention and the bare-link `:not([class*="l-ui-"])` rule in CSS.md so future contributors do not "fix" them into BEM elements.
ReviewProject-wide BEM tightening: every component modifier is now a true delta (no longer re-applies base styles via No issues found. ✅ Ship - The refactor is internally consistent, all engine views and helpers have been updated in lock-step, the JS controllers match the new CSS selectors, and the migration path is clearly documented. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.