Replace modifier mode with multi-modifier support in all modes - #209
Merged
Conversation
Any non-root row can now be promoted to a sub-block root by clicking its type badge (ELEM → BLOCK). The row and all its descendants adopt a new BEM block scope, so a whole section can be reBEMed with distinct block prefixes (hero__*, card__*, etc.) in one Apply. Each row also gains a row.modifiers string[] field (replaces the old single modifier string). In add/rename/replace mode a --modifier placeholder input is shown per row; typing fills it in and an '+ Add modifier' button appears to attach additional modifiers. All non-empty modifiers are attached alongside the primary class. The standalone 'Add modifier' mode is removed — modifier attachment is now an inline feature of the three class-mutation modes. Implementation notes: - computeBlockAssignment(): new exported pure helper; stack-based depth walk that resolves each row to its owning block name. Shared by buildPlan() and BemPanel's rowBlockNames derived. - buildPlan(): uses computeBlockAssignment, validates all block roots, populates op.modifierSlugs after auto-numbering so modifier class names always reference the final (post-numbered) base name. - applyToSubtree(): appends modifier classes from op.modifierSlugs to nextIds before setElementClasses; uses per-op op.blockName for labelFromClass instead of a single root blockName. - BemPanel: rowBlockNames derived feeds per-row blockName to Row; previewClassNames now Map<id, string[]> covering base + modifiers. - Row: type badge is a clickable toggle for non-root rows; modifier section supports dynamic list with add/remove. https://claude.ai/code/session_01MR5eh7i7GRVpvudPrCjm2w
|
Warning Review limit reached
More reviews will be available in 53 minutes and 21 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR refactors the reBEMer BEM-class mutation system by removing the "modifier" operational mode and introducing a ChangesBEM Plan Model and Application Refactor
Sequence DiagramsequenceDiagram
participant User as User / Panel
participant buildPlan as buildPlan()
participant BlockAssign as computeBlockAssignment()
participant Validate as validateName()
participant Number as applyAutoNumbering()
participant applyTo as applyToSubtree()
participant GlobalClasses as Global Class IDs
User->>buildPlan: rows, rootId, mode
buildPlan->>Validate: validate root + sub-block names
buildPlan->>BlockAssign: compute per-row owning block
BlockAssign-->>buildPlan: rowBlockNames map
buildPlan->>Number: auto-number ops
Number-->>buildPlan: ops with numbered finalClass
buildPlan->>buildPlan: resolve modifiers to modifierSlugs
buildPlan-->>User: ops array
User->>applyTo: ops, mode, syncLabels
applyTo->>applyTo: by mode, compute primary class IDs
applyTo->>GlobalClasses: upser primary classes
applyTo->>GlobalClasses: append modifier classes per modifierSlugs
applyTo->>applyTo: sync labels if enabled
applyTo-->>User: done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/lib/apply.js (1)
110-121: 💤 Low valueMinor JSDoc clarification: include handling is asymmetric.
The docstring says the function determines ownership "for each included row," but the implementation assigns block ownership to all non-block element rows regardless of
includestatus (lines 138-141). This is intentional for UI preview purposes (so excluded rows still show their would-be prefix), but the documentation could be clearer about this nuance.Consider updating:
- * Walk rows in document order to determine the owning BEM block for each - * included row. + * Walk rows in document order to determine the owning BEM block for each row. + * Block/sub-block roots are only pushed onto the stack when included; + * element rows are assigned regardless of include status (for UI preview).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/lib/apply.js` around lines 110 - 121, Update the JSDoc for the exported function that walks rows (parameters rows and rootId, returning Map row.id → owning block's slugified name) to explicitly state that ownership is assigned to all non-block element rows regardless of their include flag (this asymmetry is intentional to support UI preview/prefix display used by BemPanel), i.e., clarify the function determines the "would-be" owning BEM block for excluded rows as well rather than limiting results to included rows.plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/Row.svelte (1)
134-134: 💤 Low valueAvoid keying the modifier list by array index.
{#eachrow.modifiers as _mod, mi (mi)}keys by position while the inputs usebind:value={row.modifiers[mi]}. Removing a middle entry reuses DOM nodes by index, so the surviving inputs rebind to shifted values and can carry over stale focus/IME/selection state. Consider keying by a stable per-modifier id instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/Row.svelte` at line 134, The each block in Row.svelte currently keys modifiers by index ({`#each` row.modifiers as _mod, mi (mi)}) which causes DOM reuse and input state bleeding when items are removed; change the data model so each modifier is an object with a stable id (e.g., {id, value, ...}), update the each to key by that id (use _mod.id instead of mi) and update the input bindings to use the loop variable (bind:value={_mod.value} or equivalent) rather than indexing into row.modifiers by mi so DOM nodes stay tied to stable modifier identities.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/Row.svelte`:
- Around line 136-145: The modifier input currently calls markAsUserTyped
(oninput={markAsUserTyped}), which flips row.suggestedFrom to 'user' and
incorrectly locks the base name; remove that side-effect by not invoking
markAsUserTyped from the modifier field (bind:value={row.modifiers[mi]} on the
input). Instead, either remove the oninput from the modifier input entirely or
replace it with a no-op/modifier-only handler that updates the modifier value
without touching row.suggestedFrom; keep markAsUserTyped only on the base name
input so that only edits to the base class set row.suggestedFrom and block
auto-numbering.
---
Nitpick comments:
In
`@plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/Row.svelte`:
- Line 134: The each block in Row.svelte currently keys modifiers by index
({`#each` row.modifiers as _mod, mi (mi)}) which causes DOM reuse and input state
bleeding when items are removed; change the data model so each modifier is an
object with a stable id (e.g., {id, value, ...}), update the each to key by that
id (use _mod.id instead of mi) and update the input bindings to use the loop
variable (bind:value={_mod.value} or equivalent) rather than indexing into
row.modifiers by mi so DOM nodes stay tied to stable modifier identities.
In `@plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/lib/apply.js`:
- Around line 110-121: Update the JSDoc for the exported function that walks
rows (parameters rows and rootId, returning Map row.id → owning block's
slugified name) to explicitly state that ownership is assigned to all non-block
element rows regardless of their include flag (this asymmetry is intentional to
support UI preview/prefix display used by BemPanel), i.e., clarify the function
determines the "would-be" owning BEM block for excluded rows as well rather than
limiting results to included rows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 83cc4c73-c2d2-4696-b042-f3799ac5c12c
📒 Files selected for processing (4)
plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/BemPanel.svelteplugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/Row.svelteplugins/SLASHED-for-WP/integrations/bricks/editor-app/src/lib/apply.jsplugins/SLASHED-for-WP/integrations/bricks/editor-app/src/styles/panel.css
… JSDoc Typing into a modifier input was calling markAsUserTyped(), which flipped row.suggestedFrom to 'user' and silently locked the base name out of sibling auto-numbering — even though the user never touched the base name input. Modifier slugs are resolved after numbering and never affect the base class identity, so the oninput handler is removed from the modifier inputs. Also clarifies the computeBlockAssignment() JSDoc: element rows are assigned an owner regardless of their include status (intentional, for UI prefix preview), which was previously undocumented. https://claude.ai/code/session_01MR5eh7i7GRVpvudPrCjm2w
jackgranatowski
pushed a commit
that referenced
this pull request
Jun 3, 2026
Compiles the Svelte source changes from PR #209 into the deployed assets — sub-block root promotion, inline multi-modifier inputs, and removal of the standalone modifier mode. https://claude.ai/code/session_01MR5eh7i7GRVpvudPrCjm2w
5 tasks
jackgranatowski
pushed a commit
that referenced
this pull request
Jun 3, 2026
Compiles the Svelte source changes from PR #209 into the deployed assets — sub-block root promotion, inline multi-modifier inputs, and removal of the standalone modifier mode. https://claude.ai/code/session_01MR5eh7i7GRVpvudPrCjm2w
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
2 participants
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.
Summary
This PR removes the dedicated "modifier" mode and instead adds support for multiple modifiers across all non-migrate modes (add, rename, replace). Modifiers are now specified as an array on each row and produce additional
--modifierclasses alongside the primary class. Additionally, introduces support for sub-block roots, allowing non-root elements to be promoted to their own BEM block scope.Key Changes
Removed modifier mode: The
'modifier'mode is no longer a valid option. Modifier functionality is now available in add/rename/replace modes viarow.modifiers: string[].Multi-modifier support: Each row can now carry multiple modifiers in an array. Each non-empty modifier slug produces an additional
finalClass--modSlugclass attached alongside the primary class. Modifier slugs are resolved after auto-numbering to ensure they reference the correct post-numbered base class name.Sub-block roots: Non-root rows can now be promoted to block roots via
row.isBlockRoot: true. A sub-block root'sfinalClassbecomes its own block name (notparentBlock__elemName), and all its descendants adopt it as their owning block. Implemented via newcomputeBlockAssignment()function that walks rows with a depth-based stack.Updated Row component:
Updated BemPanel component:
syncLabelsdisable for modifier mode (now always available)computeBlockAssignment()to derive per-row block names for prefix displayUpdated apply.js logic:
'modifier'case from the mode switch inapplyToSubtree()Implementation Details
buildPlan(), after auto-numbering, ensuring modifiers reference the final (post-numbered) class names.computeBlockAssignment()function is exported so BemPanel can reuse it for live prefix display without re-running the full plan.buildPlan()with a combined error message listing all invalid names.https://claude.ai/code/session_01MR5eh7i7GRVpvudPrCjm2w
Summary by CodeRabbit
New Features
UI/UX Changes