Support grid layouts in the STACK flow element - #4365
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSTACK rendering now uses shared grid utilities and ChangesSTACK layout rendering
Builder panel spacing
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant FlowComponentRenderer
participant StackContainer
participant getStackGridSx
participant FlowChild
FlowComponentRenderer->>StackContainer: render STACK component
StackContainer->>getStackGridSx: parse items and derive styles
getStackGridSx-->>StackContainer: grid styles or null
StackContainer->>FlowChild: render nested children and actions
FlowChild-->>FlowComponentRenderer: dispatch selected action or submit
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@frontend/packages/design/src/components/flow/__tests__/FlowComponentRenderer.test.tsx`:
- Around line 324-325: Update the gridColumnCount helper in
FlowComponentRenderer tests to parse the repeat() count from gridTemplateColumns
rather than counting whitespace-separated tokens. Ensure repeat(3, 1fr) produces
a column count of 3 and preserve the helper’s numeric return behavior.
In `@frontend/packages/design/src/components/flow/adapters/StackContainer.tsx`:
- Around line 29-31: Update coerceItems so string inputs are accepted only when
the entire value represents a valid number, rather than relying on parseInt’s
partial parsing; malformed values such as “2invalid” must return the existing
flex fallback of 1, while valid numeric values continue through finite-number
validation and flooring.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e5aa8e0-ecc3-4768-8f0a-76c0a3499a5f
📒 Files selected for processing (4)
frontend/packages/design/src/components/flow/__tests__/FlowComponentRenderer.test.tsxfrontend/packages/design/src/components/flow/adapters/BlockAdapter.tsxfrontend/packages/design/src/components/flow/adapters/StackAdapter.tsxfrontend/packages/design/src/components/flow/adapters/StackContainer.tsx
8cff9b3 to
f73b3b0
Compare
f73b3b0 to
9223917
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx (1)
183-193: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
emptySlotCountonly accounts for the first row, undercounting empty cells in later rows.let emptySlotCount: number; if (useGrid) { emptySlotCount = Math.max(0, (items ?? 0) - filteredComponents.length); } else { emptySlotCount = filteredComponents.length === 0 ? 1 : 0; }
With
items=2and 3 children (2 filled in row 1, 1 in row 2), this yieldsmax(0, 2-3)=0, hiding the empty cell in row 2 that the comment above says should show a "Drop here" placeholder. The bug only surfaces when child count exceedsitemsand isn't an exact multiple — exactly the multi-row auto-flow scenario this PR introduces — and isn't covered by the current tests (items: 1, oritemsabsent).🐛 Proposed fix accounting for a partially-filled last row
let emptySlotCount: number; if (useGrid) { - emptySlotCount = Math.max(0, (items ?? 0) - filteredComponents.length); + const columns = items ?? 1; + const totalCells = Math.ceil((filteredComponents.length || columns) / columns) * columns; + emptySlotCount = Math.max(0, totalCells - filteredComponents.length); } else { emptySlotCount = filteredComponents.length === 0 ? 1 : 0; }Want me to add a regression test for
items=2with 3/5 children to lock this in?🤖 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 `@frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx` around lines 183 - 193, Update the useGrid branch calculating emptySlotCount in StackAdapter so it accounts for every partially filled final row, not just total capacity minus all children. Preserve zero placeholders for completely filled rows, add the remaining cells needed to complete the last row when filteredComponents.length is not divisible by items, and retain the existing flex-mode behavior.frontend/packages/design/src/components/flow/adapters/BlockAdapter.tsx (1)
179-180: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winEm dash used in code comments across two files — violates the repo's punctuation guideline. As per coding guidelines,
**/*: "Do not use em dashes (—) or double hyphens (--) in copy or UI strings, including i18n locale content; use a comma, period, or rephrasing instead." All three sites use "—" in an inline comment; replace with a period/comma or rephrase.
frontend/packages/design/src/components/flow/adapters/BlockAdapter.tsx#L179-L180: reword "STACK is a pure layout container — render it in place..." to drop the em dash.frontend/packages/design/src/components/flow/adapters/BlockAdapter.tsx#L398-L399: reword "Actions may sit inside STACK layout containers — look through them..." to drop the em dash.frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx#L183-L184: reword "always fill defined slots — show placeholders for every unoccupied slot" to drop the em dash.🤖 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 `@frontend/packages/design/src/components/flow/adapters/BlockAdapter.tsx` around lines 179 - 180, Remove the em dash from the inline comments at frontend/packages/design/src/components/flow/adapters/BlockAdapter.tsx lines 179-180 and 398-399, and frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx lines 183-184. Rephrase each comment using a period, comma, or equivalent wording while preserving its meaning.Source: Coding guidelines
🧹 Nitpick comments (1)
frontend/apps/console/src/features/flows/components/resources/elements/adapters/__tests__/StackAdapter.test.tsx (1)
159-186: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for multi-row grids with a partial last row.
These tests only cover
items: 1anditemsabsent. Neither exercisesitems >= 2with a child count that isn't an exact multiple (e.g.,items: 2with 3 children), which is the scenario where theemptySlotCountcalculation inStackAdapter.tsxundercounts empty placeholders (see comment on that file).🤖 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 `@frontend/apps/console/src/features/flows/components/resources/elements/adapters/__tests__/StackAdapter.test.tsx` around lines 159 - 186, The Grid mode tests in StackAdapter.test.tsx lack coverage for a multi-row grid with a partially filled final row. Add a test using items: 2 and three children, render StackAdapter, and assert the rendered children plus the expected empty “Drop here” placeholder count based on the remaining slot in the final row.
🤖 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
`@frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx`:
- Line 196: Update the flexDirection fallback in StackAdapter so an omitted
stackElement.direction defaults to 'column', matching StackContainer’s runtime
default while preserving explicitly provided directions.
In `@frontend/packages/design/src/components/flow/adapters/StackContainer.tsx`:
- Around line 34-58: Align the defaults used by getStackGridSx with
StackContainer’s flex branch: an unset direction must produce a column-oriented
layout, while unset align and justify must match center and flex-start. Update
the grid-style generation in getStackGridSx, preserving explicit direction,
align, and justify values so enabling items does not otherwise alter layout.
In `@frontend/packages/design/src/utils/getStackGridSx.ts`:
- Around line 44-50: Update parseStackItems so single-item values are rejected
by changing the minimum accepted slots threshold from 1 to 2, preserving flex
layout for values below 2 and returning undefined for them.
---
Outside diff comments:
In
`@frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx`:
- Around line 183-193: Update the useGrid branch calculating emptySlotCount in
StackAdapter so it accounts for every partially filled final row, not just total
capacity minus all children. Preserve zero placeholders for completely filled
rows, add the remaining cells needed to complete the last row when
filteredComponents.length is not divisible by items, and retain the existing
flex-mode behavior.
In `@frontend/packages/design/src/components/flow/adapters/BlockAdapter.tsx`:
- Around line 179-180: Remove the em dash from the inline comments at
frontend/packages/design/src/components/flow/adapters/BlockAdapter.tsx lines
179-180 and 398-399, and
frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx
lines 183-184. Rephrase each comment using a period, comma, or equivalent
wording while preserving its meaning.
---
Nitpick comments:
In
`@frontend/apps/console/src/features/flows/components/resources/elements/adapters/__tests__/StackAdapter.test.tsx`:
- Around line 159-186: The Grid mode tests in StackAdapter.test.tsx lack
coverage for a multi-row grid with a partially filled final row. Add a test
using items: 2 and three children, render StackAdapter, and assert the rendered
children plus the expected empty “Drop here” placeholder count based on the
remaining slot in the final row.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7045e0e4-e8a8-4a81-b152-4003c84bc2ca
📒 Files selected for processing (11)
frontend/apps/console/src/features/flows/components/resource-property-panel/CommonElementPropertyFactory.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/__tests__/CommonElementPropertyFactory.test.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/__tests__/StackAdapter.test.tsxfrontend/packages/design/src/components/flow/__tests__/FlowComponentRenderer.test.tsxfrontend/packages/design/src/components/flow/adapters/BlockAdapter.tsxfrontend/packages/design/src/components/flow/adapters/StackAdapter.tsxfrontend/packages/design/src/components/flow/adapters/StackContainer.tsxfrontend/packages/design/src/index.tsfrontend/packages/design/src/utils/__tests__/getStackGridSx.test.tsfrontend/packages/design/src/utils/getStackGridSx.ts
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
9223917 to
549243f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@frontend/packages/design/src/components/flow/__tests__/FlowComponentRenderer.test.tsx`:
- Line 436: Update the existing track-count helper in FlowComponentRenderer
tests to accept a template string and correctly parse repeat expressions such as
repeat(2, 1fr). Replace the rows assertion’s whitespace-based split with this
helper, and reuse the same helper for both grid columns and rows.
In `@frontend/packages/design/src/utils/getStackGridSx.ts`:
- Around line 41-49: Update the JSDoc for parseStackItems to accurately state
that finite numeric inputs are floored before validating they produce at least
one slot, matching the current implementation and tests where 2.7 becomes 2. Do
not change the parser behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 60312460-e359-4cf5-a818-9dbb28e166c7
📒 Files selected for processing (11)
frontend/apps/console/src/features/flows/components/resource-property-panel/CommonElementPropertyFactory.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/__tests__/CommonElementPropertyFactory.test.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/__tests__/StackAdapter.test.tsxfrontend/packages/design/src/components/flow/__tests__/FlowComponentRenderer.test.tsxfrontend/packages/design/src/components/flow/adapters/BlockAdapter.tsxfrontend/packages/design/src/components/flow/adapters/StackAdapter.tsxfrontend/packages/design/src/components/flow/adapters/StackContainer.tsxfrontend/packages/design/src/index.tsfrontend/packages/design/src/utils/__tests__/getStackGridSx.test.tsfrontend/packages/design/src/utils/getStackGridSx.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- frontend/packages/design/src/index.ts
- frontend/packages/design/src/components/flow/adapters/StackContainer.tsx
- frontend/apps/console/src/features/flows/components/resource-property-panel/CommonElementPropertyFactory.tsx
- frontend/packages/design/src/components/flow/adapters/BlockAdapter.tsx
- frontend/packages/design/src/utils/tests/getStackGridSx.test.ts
- frontend/apps/console/src/features/flows/components/resource-property-panel/tests/CommonElementPropertyFactory.test.tsx
- frontend/packages/design/src/components/flow/adapters/StackAdapter.tsx
- frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx
549243f to
0f55856
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/apps/console/src/features/flows/components/resource-property-panel/TextPropertyField.tsx (1)
151-186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd fallback default strings to the reworked
t()calls.The tooltip and placeholder lookups were rewritten in this hunk without a fallback default.
As per coding guidelines: "Every
t('key')call must provide a fallback default string, either as the positional fallback argument or throughdefaultValuein the options object".♻️ Proposed change
- <Tooltip title={t('flows:core.elements.textPropertyField.tooltip.configureDynamicValue')}> + <Tooltip + title={t('flows:core.elements.textPropertyField.tooltip.configureDynamicValue', 'Configure dynamic value')} + > @@ const placeholder: string = t('flows:core.elements.textPropertyField.placeholder', { + defaultValue: 'Enter {{propertyName}}', propertyName: startCase(propertyKey), });🤖 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 `@frontend/apps/console/src/features/flows/components/resource-property-panel/TextPropertyField.tsx` around lines 151 - 186, Update the translation calls for the dynamic-value tooltip and text placeholder in the TextPropertyField component to include explicit fallback default strings, using either the positional fallback argument or the defaultValue option while preserving the existing interpolation for propertyName.Source: Coding guidelines
🤖 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
`@frontend/apps/console/src/features/flows/components/resource-property-panel/TextPropertyField.tsx`:
- Around line 222-268: Update the Autocomplete branch in the TextPropertyField
render to assign its underlying input the same `${resource.id}-${propertyKey}`
id targeted by FormLabel, preserving the label association and accessible name.
Also forward the existing rest props to the Autocomplete-rendered TextField,
matching the plain textField path instead of dropping caller-supplied
properties.
In
`@frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx`:
- Around line 36-43: Update the StackElement documentation for the items
property to state that CSS Grid activates only when items is a valid value of at
least two; clarify that 1, malformed, and non-positive values preserve the flex
layout.
- Around line 142-144: Update the direction handling in StackAdapter’s grid-mode
controls to use the same condition as getStackGridSx: treat every direction
other than “column” as row-oriented. Add the custom-direction case so reorder
controls match the rendered grid axis, while preserving existing behavior for
explicit “row” and “column” values.
---
Nitpick comments:
In
`@frontend/apps/console/src/features/flows/components/resource-property-panel/TextPropertyField.tsx`:
- Around line 151-186: Update the translation calls for the dynamic-value
tooltip and text placeholder in the TextPropertyField component to include
explicit fallback default strings, using either the positional fallback argument
or the defaultValue option while preserving the existing interpolation for
propertyName.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 09a2736b-72a9-4573-9baf-982741f8924b
📒 Files selected for processing (13)
frontend/apps/console/src/features/flows/components/resource-property-panel/CommonElementPropertyFactory.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/TextPropertyField.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/__tests__/CommonElementPropertyFactory.test.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/__tests__/TextPropertyField.test.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/__tests__/StackAdapter.test.tsxfrontend/packages/design/src/components/flow/__tests__/FlowComponentRenderer.test.tsxfrontend/packages/design/src/components/flow/adapters/BlockAdapter.tsxfrontend/packages/design/src/components/flow/adapters/StackAdapter.tsxfrontend/packages/design/src/components/flow/adapters/StackContainer.tsxfrontend/packages/design/src/index.tsfrontend/packages/design/src/utils/__tests__/getStackGridSx.test.tsfrontend/packages/design/src/utils/getStackGridSx.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- frontend/packages/design/src/index.ts
- frontend/packages/design/src/utils/getStackGridSx.ts
- frontend/packages/design/src/components/flow/adapters/StackContainer.tsx
- frontend/apps/console/src/features/flows/components/resource-property-panel/tests/CommonElementPropertyFactory.test.tsx
- frontend/packages/design/src/components/flow/adapters/StackAdapter.tsx
- frontend/packages/design/src/components/flow/adapters/BlockAdapter.tsx
bbc3498 to
c5466cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx (2)
162-170: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
isRowstill diverges fromgetStackGridSx's axis logic for custom directions.
getStackGridSxtreats every direction other than'column'as row-oriented, butisRowhere only treats exactly'row'as row-oriented. For a custom/non-standarddirectionvalue, the grid renders row-oriented while reorder controls (Move Up/Down vs Left/Right) show the column-oriented set. A prior review flagged this same code and marked it "Addressed in commit c5466cd," yet the code is unchanged.🐛 Proposed fix
- const isRow = (stackElement?.direction ?? 'row') === 'row'; + const isRow = useGrid + ? stackElement?.direction !== 'column' + : (stackElement?.direction ?? 'row') === 'row';🤖 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 `@frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx` around lines 162 - 170, Update the isRow calculation in the StackAdapter component to match getStackGridSx axis logic: treat only direction === 'column' as column-oriented and all other directions, including custom values and the default, as row-oriented. Keep reorder controls aligned with the grid’s orientation.
38-45: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winJSDoc still describes the wrong grid-activation threshold.
Line 38 says grid activates whenever
itemsis set, butparseStackItems(andgetStackGridSx) treat1,0, negative, and malformed values as flex mode — grid only kicks in for a valid value of at least two. A prior review on this exact file/line range flagged this and it was marked "Addressed in commit c5466cd," but the current code still has the pre-fix text.🐛 Proposed fix
- * When `items` is set, the stack uses CSS Grid instead of flexbox. + * When `items` is a valid value of at least two, the stack uses CSS Grid instead of flexbox.🤖 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 `@frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx` around lines 38 - 45, Update the StackElement JSDoc to state that CSS Grid activates only when items is a valid value of at least two; zero, one, negative, missing, or malformed values retain the flex layout. Keep the change limited to the documentation above StackElement.
🧹 Nitpick comments (3)
frontend/apps/console/src/features/login-flow/hooks/useElementAddition.ts (1)
198-247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
formIdtocontainerIdfor consistency.The public type (line 47) already renamed this parameter to
containerIdto reflect that it now targets any container (form or stack), but the implementation still usesformIdinternally.♻️ Proposed rename
- (element: Element, formId: string): void => { + (element: Element, containerId: string): void => { // Use generateStepElement to properly apply variants and generate unique IDs const generatedElement: Element = generateStepElement(element); let viewStepId: string | null = null; setNodes((prevNodes: Node[]) => { const existingViewStep = prevNodes.find((node) => { if (node.type !== StepTypes.View) return false; const nodeData = node.data as {components?: Element[]} | undefined; - return containsComponent(nodeData?.components ?? [], formId); + return containsComponent(nodeData?.components ?? [], containerId); });(and similarly for the
appendToComponent(existingComponents, formId, generatedElement)call.)🤖 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 `@frontend/apps/console/src/features/login-flow/hooks/useElementAddition.ts` around lines 198 - 247, Rename the handleAddElementToForm parameter and all internal references from formId to containerId, including containsComponent and appendToComponent calls, so the implementation matches the public type and supports any container.frontend/apps/console/src/features/flows/components/resources/steps/view/__tests__/ReorderableElement.test.tsx (1)
332-372: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a regression test for late-loading
availableElements.None of the new stack tests re-render with an updated
availableElementsprop after mount, so they wouldn't catch theformCompatibleElementsstaleness issue flagged inReorderableElement.tsx(lines 320-332). A rerender-based test (render with an empty list, thenrerenderwithmockStackElements, then assert the menu now shows the newly available item) would pin down the fix.🤖 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 `@frontend/apps/console/src/features/flows/components/resources/steps/view/__tests__/ReorderableElement.test.tsx` around lines 332 - 372, Update the Stack-element test coverage around ReorderableElement to verify late-loading availableElements: initially render with an empty list, rerender with mockStackElements, open the add-component handle, and assert the newly available stack-compatible item appears. Use the existing ReorderableElement test setup and preserve the current compatibility assertions.frontend/apps/console/src/features/flows/components/resources/steps/view/ReorderableElement.tsx (1)
322-332: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame "addable elements" filter copy-pasted in three places. Each site independently re-implements
allowedTypes.includes(element.type) && element.display?.showOnResourcePanel !== false, only varying theallowedTypesconstant; a future change to the visibility rule (e.g. adding adeprecatedcheck) would need to be applied in all three, and one is easy to miss.
frontend/apps/console/src/features/flows/components/resources/steps/view/ReorderableElement.tsx#L322-L332: extract a sharedgetAddableElements(elements, allowedTypes)helper and call it here with the already-computedallowedTypes.frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx#L222-L226: replace the inline filter with the same shared helper, passingVisualFlowConstants.FLOW_BUILDER_STACK_ALLOWED_RESOURCE_TYPES.frontend/apps/console/src/features/flows/components/resources/elements/adapters/FormAdapter.tsx#L79-L83: replace the inline filter with the same shared helper, passingVisualFlowConstants.FLOW_BUILDER_FORM_ALLOWED_RESOURCE_TYPES.🤖 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 `@frontend/apps/console/src/features/flows/components/resources/steps/view/ReorderableElement.tsx` around lines 322 - 332, Extract a shared getAddableElements(elements, allowedTypes) helper that centralizes the allowed-type and showOnResourcePanel visibility checks. Update ReorderableElement.tsx (lines 322-332) to call it with the computed allowedTypes, StackAdapter.tsx (lines 222-226) with FLOW_BUILDER_STACK_ALLOWED_RESOURCE_TYPES, and FormAdapter.tsx (lines 79-83) with FLOW_BUILDER_FORM_ALLOWED_RESOURCE_TYPES.
🤖 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
`@frontend/apps/console/src/features/flows/components/resources/steps/view/ReorderableElement.tsx`:
- Around line 320-332: Update the formCompatibleElements useMemo in
ReorderableElement so it recomputes when availableElements changes, rather than
relying only on isContainer and isStack while reading
depsRef.current.availableElements. Preserve the existing filtering and
allowed-type selection, and use the component’s availableElements value as a
dependency/source so late-loaded updates refresh the add-menu.
---
Duplicate comments:
In
`@frontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsx`:
- Around line 162-170: Update the isRow calculation in the StackAdapter
component to match getStackGridSx axis logic: treat only direction === 'column'
as column-oriented and all other directions, including custom values and the
default, as row-oriented. Keep reorder controls aligned with the grid’s
orientation.
- Around line 38-45: Update the StackElement JSDoc to state that CSS Grid
activates only when items is a valid value of at least two; zero, one, negative,
missing, or malformed values retain the flex layout. Keep the change limited to
the documentation above StackElement.
---
Nitpick comments:
In
`@frontend/apps/console/src/features/flows/components/resources/steps/view/__tests__/ReorderableElement.test.tsx`:
- Around line 332-372: Update the Stack-element test coverage around
ReorderableElement to verify late-loading availableElements: initially render
with an empty list, rerender with mockStackElements, open the add-component
handle, and assert the newly available stack-compatible item appears. Use the
existing ReorderableElement test setup and preserve the current compatibility
assertions.
In
`@frontend/apps/console/src/features/flows/components/resources/steps/view/ReorderableElement.tsx`:
- Around line 322-332: Extract a shared getAddableElements(elements,
allowedTypes) helper that centralizes the allowed-type and showOnResourcePanel
visibility checks. Update ReorderableElement.tsx (lines 322-332) to call it with
the computed allowedTypes, StackAdapter.tsx (lines 222-226) with
FLOW_BUILDER_STACK_ALLOWED_RESOURCE_TYPES, and FormAdapter.tsx (lines 79-83)
with FLOW_BUILDER_FORM_ALLOWED_RESOURCE_TYPES.
In `@frontend/apps/console/src/features/login-flow/hooks/useElementAddition.ts`:
- Around line 198-247: Rename the handleAddElementToForm parameter and all
internal references from formId to containerId, including containsComponent and
appendToComponent calls, so the implementation matches the public type and
supports any container.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1760ef7b-a74d-4cd4-9318-75a747673fdc
📒 Files selected for processing (19)
frontend/apps/console/src/features/flows/components/resource-property-panel/CommonElementPropertyFactory.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/TextPropertyField.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/__tests__/CommonElementPropertyFactory.test.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/__tests__/TextPropertyField.test.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/FormAdapter.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/StackAdapter.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/__tests__/FormAdapter.test.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/__tests__/StackAdapter.test.tsxfrontend/apps/console/src/features/flows/components/resources/steps/view/ReorderableElement.tsxfrontend/apps/console/src/features/flows/components/resources/steps/view/__tests__/ReorderableElement.test.tsxfrontend/apps/console/src/features/login-flow/hooks/useElementAddition.tsfrontend/packages/components/src/lab/components/BuilderLayout/BuilderStaticPanel.tsxfrontend/packages/design/src/components/flow/__tests__/FlowComponentRenderer.test.tsxfrontend/packages/design/src/components/flow/adapters/BlockAdapter.tsxfrontend/packages/design/src/components/flow/adapters/StackAdapter.tsxfrontend/packages/design/src/components/flow/adapters/StackContainer.tsxfrontend/packages/design/src/index.tsfrontend/packages/design/src/utils/__tests__/getStackGridSx.test.tsfrontend/packages/design/src/utils/getStackGridSx.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- frontend/packages/design/src/components/flow/adapters/BlockAdapter.tsx
- frontend/packages/design/src/utils/tests/getStackGridSx.test.ts
- frontend/packages/design/src/index.ts
- frontend/packages/design/src/utils/getStackGridSx.ts
- frontend/packages/design/src/components/flow/adapters/StackContainer.tsx
- frontend/apps/console/src/features/flows/components/resource-property-panel/tests/TextPropertyField.test.tsx
- frontend/apps/console/src/features/flows/components/resource-property-panel/tests/CommonElementPropertyFactory.test.tsx
- frontend/packages/design/src/components/flow/adapters/StackAdapter.tsx
- frontend/apps/console/src/features/flows/components/resource-property-panel/TextPropertyField.tsx
6479809 to
cb31e9b
Compare
Honor the items property on STACK flow elements so children can be laid out in a grid. items is the number of slots across the main axis and direction picks that axis: row makes it the column count (items: 2 with four buttons renders a 2 x 2 grid, items: 1 renders one per row), and column makes it the row count. Stacks without items keep the flex layout. The flow builder canvas and the runtime renderer now share a single layout helper so the builder preview matches what the Gate renders, and the direction, align and justify property fields become searchable autocompletes that still accept custom values. Fixes thunder-id#3703
cb31e9b to
54a4882
Compare
Purpose
Resolves the inability to arrange STACK children (e.g. action buttons) in a grid at runtime. A flow defining a STACK with four buttons could only render as a strict flex row or column, while the flow builder canvas already previewed grid layouts via the
itemsproperty. This PR makes the runtime renderer honoritemsand gives the property one consistent meaning across the builder, the Gate, and the React SDK.itemsis the number of slots across the main axis, anddirectionpicks that axis:row(default)rowcolumnNot a breaking change. Grid mode requires
items >= 2. Stacks withitems: 1(which the builder seeds on every stack it creates) or noitemsat all keep their existing flex layout, so previously authored flows render exactly as before.Approach
getStackGridSxhelper in@thunderid/design, used by both the runtime renderers and the console flow builder canvas, so the builder preview and the Gate can no longer drift apart.StackAdapter, and the form and trigger block paths inBlockAdapter, which previously duplicated the layout mapping inline).itemsis parsed defensively from the SDK'sstring | numbertype: malformed values ("2invalid"), zero, and negatives fall back to flex, and the slot count is clamped so a mistyped value cannot render thousands of cells.classesbeing dropped on STACKs nested inside BLOCKs.direction,alignandjustifygain searchable suggestions while staying free text. This is implemented as an optionalsuggestionsprop on the existingTextPropertyField, so those fields keep the dynamic-value (fx) button, inline validation errors, label placement, and debounce handling that every other property field has.EmbeddedFlowComponent.itemsalready exists in the SDK.A companion fix for the
@thunderid/reactSDK's own renderer (AuthOptionFactory) is submitted to thunder-id/javascript-sdks.Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
New Features
Bug Fixes
1, including corrected placeholder behavior.Tests