feat: Introduce info_card, icon and icon_button to ui-kit #37398
feat: Introduce info_card, icon and icon_button to ui-kit #37398
info_card, icon and icon_button to ui-kit #37398Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: b3bcc3e The changes in this PR will be included in the next version bump. This PR includes changesets to release 42 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds Icon and IconButton element types and an InfoCard layout block to ui-kit and fuselage-ui-kit, implements corresponding React components and renderers, adds story payloads and playground presets, and updates package versions and changelog via a changeset. Changes
Sequence Diagram(s)sequenceDiagram
participant Surface as FuselageSurfaceRenderer
participant InfoCard as InfoCard
participant IconEl as IconElement
participant IconBtn as IconButtonElement
participant AppProv as AppIdProvider
participant Browser as Browser
note over Surface,InfoCard: Render flow for UiKit.InfoCardBlock
Surface->>InfoCard: render info_card(block, context, index)
InfoCard->>IconEl: render icon elements per row
InfoCard->>IconBtn: render IconButtonElement if row.action exists
alt IconButton has url
IconBtn->>Browser: open link (target="_blank")
else IconButton is in-app action
Surface->>AppProv: wrap IconBtn with app context
IconBtn->>Surface: send action payload on click
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-11-16T14:23:18.933ZApplied to files:
🧬 Code graph analysis (1)packages/fuselage-ui-kit/src/stories/payloads/infoCard.ts (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (3)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37398 +/- ##
===========================================
- Coverage 68.97% 68.97% -0.01%
===========================================
Files 3358 3359 +1
Lines 114215 114224 +9
Branches 20534 20535 +1
===========================================
+ Hits 78780 78785 +5
- Misses 33344 33349 +5
+ Partials 2091 2090 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
gabriellsh
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
7-11: Consider makingelementsa readonly property for consistency.The
elementsproperty is typed as a readonly array, but the property itself is not markedreadonly. In contrast,InfoCardBlock.rowson line 15 uses bothreadonlyfor the property and the array type:rows: readonly InfoCardRow[];For consistent immutability patterns, consider:
type InfoCardRow = { background: 'default' | 'secondary'; - elements: readonly (IconElement | PlainText)[]; + readonly elements: readonly (IconElement | PlainText)[]; action?: IconButtonElement; };This prevents both reassignment (
row.elements = newArray) and mutation (row.elements[0] = newValue), aligning with the pattern used forInfoCardBlock.rows.packages/ui-kit/src/blocks/elements/IconElement.ts (2)
1-1: Consider exporting AvailableIcons type for external validation.The
AvailableIconstype could be useful for consumers who need to validate icon names or build tooling around the icon set. Consider exporting it alongsideIconElement.Apply this diff to export the type:
-type AvailableIcons = 'phone-off' | 'phone-issue' | 'clock' | 'arrow-forward' | 'info'; +export type AvailableIcons = 'phone-off' | 'phone-issue' | 'clock' | 'arrow-forward' | 'info';
3-7: Consider adding accessibility support for IconElement.The current IconElement type lacks accessibility attributes like
labeloraria-label. While icons often don't require labels when used decoratively, actionable or informative icons benefit from them for screen readers.Consider extending the type:
export type IconElement = { type: 'icon'; icon: AvailableIcons; variant: 'default' | 'danger'; + label?: string; };packages/fuselage-ui-kit/src/blocks/InfoCard.tsx (4)
20-20: Consider responsive design for maxWidth.The hardcoded
maxWidth='345px'may not adapt well to different screen sizes. Consider using responsive units or Fuselage's responsive prop patterns.- maxWidth='345px' + maxWidth={{ base: '100%', sm: '345px' }}
29-30: Handle unexpected background values explicitly.The current logic only checks for
background === 'default'. If other background values exist (e.g., 'secondary' as seen in related payloads), they are silently ignored. Consider explicitly handling all expected values.- backgroundColor={background === 'default' ? 'surface-light' : undefined} + backgroundColor={ + background === 'default' + ? 'surface-light' + : background === 'secondary' + ? 'surface-neutral' + : undefined + }Based on the payload examples in the related code snippets, rows can have
background: 'secondary'(see apps/uikit-playground/src/Payload/actionBlock/infoCard/index.ts line 41).
37-53: Ensure unique keys for elements within a row.Using only the array index as a key can lead to issues if elements are reordered or if multiple rows contain similar elements. Consider combining the row index with the element index for better uniqueness.
- {elements.map((element, index) => { + {elements.map((element, elementIndex) => { if (element.type === 'icon') { return ( - <Box key={index} mi={4}> - <IconElement block={element} context={context} surfaceRenderer={surfaceRenderer} index={index} /> + <Box key={`${index}-${elementIndex}`} mi={4}> + <IconElement block={element} context={context} surfaceRenderer={surfaceRenderer} index={elementIndex} /> </Box> ); } if (element.type === 'plain_text') { return ( - <Box key={index} mi={4}> - <>{surfaceRenderer.renderTextObject(element, index, UiKit.BlockContext.NONE)}</> + <Box key={`${index}-${elementIndex}`} mi={4}> + <>{surfaceRenderer.renderTextObject(element, elementIndex, UiKit.BlockContext.NONE)}</> </Box> ); }Note: Rename the inner
indexparameter toelementIndexto avoid shadowing the outerindexfrom the row mapping.
55-57: Use Box instead of div for consistency.The action is wrapped in a
divwhile the rest of the component usesBoxcomponents. Consider usingBoxfor consistency with the Fuselage component system.- <div> + <Box> {action ? <IconButtonElement block={action} context={context} surfaceRenderer={surfaceRenderer} index={index} /> : null} - </div> + </Box>apps/uikit-playground/src/Payload/actionBlock/infoCard/index.ts (1)
3-10: Constrainiconparameter so it can’t override structural fieldsRight now
iconisPartial<IconElement>and is spread last, so a caller could overridetype(or other structural fields), creating invalid payloads.You can keep flexibility for
icon/variantbut prevent changingtypeby narrowing the type:-const getIconButtonPayload = (icon: Partial<IconElement>, label?: string): IconButtonElement => ({ +const getIconButtonPayload = ( + icon: Partial<Omit<IconElement, 'type'>>, + label?: string, +): IconButtonElement => ({packages/fuselage-ui-kit/src/stories/payloads/infoCard.ts (1)
3-10: Preventiconoverrides of core IconElement fields in stories helperSame as in the playground helper,
iconisPartial<UiKit.IconElement>and is spread last, so a caller could changetypeunexpectedly.Consider tightening the type:
-const getIconButtonPayload = (icon: Partial<UiKit.IconElement>, label?: string): UiKit.IconButtonElement => ({ +const getIconButtonPayload = ( + icon: Partial<Omit<UiKit.IconElement, 'type'>>, + label?: string, +): UiKit.IconButtonElement => ({This keeps the story API flexible while avoiding structurally invalid icon elements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (19)
.changeset/hip-paws-beam.md(1 hunks)apps/uikit-playground/src/Payload/actionBlock/BlocksTree.ts(2 hunks)apps/uikit-playground/src/Payload/actionBlock/infoCard/index.ts(1 hunks)packages/fuselage-ui-kit/src/blocks/InfoCard.tsx(1 hunks)packages/fuselage-ui-kit/src/elements/IconButtonElement.tsx(1 hunks)packages/fuselage-ui-kit/src/elements/IconElement.tsx(1 hunks)packages/fuselage-ui-kit/src/stories/Message.stories.tsx(1 hunks)packages/fuselage-ui-kit/src/stories/payloads/index.ts(1 hunks)packages/fuselage-ui-kit/src/stories/payloads/infoCard.ts(1 hunks)packages/fuselage-ui-kit/src/surfaces/FuselageMessageSurfaceRenderer.tsx(1 hunks)packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx(3 hunks)packages/ui-kit/src/blocks/ActionableElement.ts(2 hunks)packages/ui-kit/src/blocks/BlockElement.ts(2 hunks)packages/ui-kit/src/blocks/LayoutBlock.ts(2 hunks)packages/ui-kit/src/blocks/LayoutBlockType.ts(1 hunks)packages/ui-kit/src/blocks/elements/IconButtonElement.ts(1 hunks)packages/ui-kit/src/blocks/elements/IconElement.ts(1 hunks)packages/ui-kit/src/blocks/layout/InfoCardBlock.ts(1 hunks)packages/ui-kit/src/index.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (13)
packages/ui-kit/src/blocks/elements/IconElement.ts (1)
packages/ui-kit/src/index.ts (1)
IconElement(31-31)
packages/ui-kit/src/blocks/BlockElement.ts (3)
packages/ui-kit/src/index.ts (3)
ExperimentalTabElement(30-30)IconElement(31-31)IconButtonElement(32-32)packages/ui-kit/src/blocks/elements/IconElement.ts (1)
IconElement(3-7)packages/ui-kit/src/blocks/elements/IconButtonElement.ts (1)
IconButtonElement(4-10)
packages/fuselage-ui-kit/src/blocks/InfoCard.tsx (2)
packages/fuselage-ui-kit/src/utils/BlockProps.ts (1)
BlockProps(5-11)packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
InfoCardBlock(13-16)
packages/ui-kit/src/blocks/elements/IconButtonElement.ts (1)
packages/ui-kit/src/blocks/elements/IconElement.ts (1)
IconElement(3-7)
packages/ui-kit/src/blocks/LayoutBlock.ts (2)
packages/ui-kit/src/index.ts (2)
ExperimentalTabNavigationBlock(45-45)InfoCardBlock(46-46)packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
InfoCardBlock(13-16)
packages/ui-kit/src/blocks/ActionableElement.ts (2)
packages/ui-kit/src/index.ts (2)
ExperimentalTabElement(30-30)IconButtonElement(32-32)packages/ui-kit/src/blocks/elements/IconButtonElement.ts (1)
IconButtonElement(4-10)
apps/uikit-playground/src/Payload/actionBlock/infoCard/index.ts (3)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (1)
icon(357-363)packages/ui-kit/src/index.ts (3)
IconElement(31-31)IconButtonElement(32-32)LayoutBlock(43-43)packages/ui-kit/src/blocks/LayoutBlock.ts (1)
LayoutBlock(14-26)
packages/fuselage-ui-kit/src/stories/payloads/infoCard.ts (3)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (1)
icon(357-363)packages/ui-kit/src/index.ts (3)
IconElement(31-31)IconButtonElement(32-32)InfoCardBlock(46-46)packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
InfoCardBlock(13-16)
packages/fuselage-ui-kit/src/elements/IconButtonElement.tsx (2)
packages/fuselage-ui-kit/src/utils/BlockProps.ts (1)
BlockProps(5-11)packages/fuselage-ui-kit/src/hooks/useUiKitState.ts (1)
useUiKitState(27-149)
apps/uikit-playground/src/Payload/actionBlock/BlocksTree.ts (1)
apps/uikit-playground/src/Payload/actionBlock/infoCard/index.ts (2)
infoCardPlain(13-27)infoCardMultipleRows(29-47)
packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
packages/ui-kit/src/index.ts (5)
IconElement(31-31)PlainText(5-5)IconButtonElement(32-32)InfoCardBlock(46-46)LayoutBlockType(53-53)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (3)
packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
InfoCardBlock(13-16)packages/ui-kit/src/index.ts (4)
InfoCardBlock(46-46)BlockContext(56-56)IconElement(31-31)IconButtonElement(32-32)packages/fuselage-ui-kit/src/stories/Message.stories.tsx (1)
InfoCard(118-118)
packages/fuselage-ui-kit/src/elements/IconElement.tsx (1)
packages/fuselage-ui-kit/src/utils/BlockProps.ts (1)
BlockProps(5-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (21)
.changeset/hip-paws-beam.md (1)
1-7: LGTM!The changeset correctly specifies minor version bumps for the affected packages, which is appropriate for adding new UI-kit elements. The changelog entry clearly describes the new features.
packages/ui-kit/src/blocks/LayoutBlockType.ts (1)
13-13: LGTM!The new
INFO_CARDenum member follows the established naming and value conventions. Placement at the end is appropriate for new additions.packages/ui-kit/src/blocks/elements/IconButtonElement.ts (1)
4-10: Verify the design intent forurlandvalueoptionality.The
IconButtonElementtype allows bothurlandvalueto be optional and potentially both present simultaneously. Typically:
urlis used for external navigation (link behavior)valueis used for internal actions (event payload)Consider whether these should be:
- Mutually exclusive (either
urlORvalue, not both)- At least one required (prevent buttons with no action)
- Intentionally flexible (document the precedence/behavior when both are present)
Please clarify the intended usage pattern. If both can be present, document which takes precedence. If they should be mutually exclusive, consider refactoring to union types:
export type IconButtonElement = | Actionable<{ type: 'icon_button'; icon: IconElement; label?: string; url: string; }> | Actionable<{ type: 'icon_button'; icon: IconElement; label?: string; value: string; }>;packages/fuselage-ui-kit/src/stories/payloads/index.ts (1)
10-10: LGTM!The barrel export follows the established pattern and properly exposes the new InfoCard payloads.
packages/ui-kit/src/index.ts (2)
31-32: LGTM!The new element exports are properly placed in the elements section and follow the established export pattern.
46-46: LGTM!The
InfoCardBlockexport is correctly placed in the layout blocks section and follows the established pattern.packages/fuselage-ui-kit/src/surfaces/FuselageMessageSurfaceRenderer.tsx (1)
10-10: LGTM!Adding
'info_card'to the allowed blocks list properly enables InfoCard rendering in message surfaces. The string literal correctly matches theLayoutBlockType.INFO_CARDenum value.packages/ui-kit/src/blocks/BlockElement.ts (2)
7-8: LGTM!The new imports are properly ordered alphabetically and follow the established type import pattern.
42-44: LGTM!The new icon element types are correctly added to the
BlockElementunion, enabling their use in block compositions. Placement at the end is appropriate for new additions.packages/ui-kit/src/blocks/ActionableElement.ts (1)
7-7: LGTM! IconButtonElement correctly added to ActionableElement union.The import and type addition follow the existing pattern and correctly extend the ActionableElement union to support icon button interactions.
Also applies to: 39-40
packages/fuselage-ui-kit/src/stories/Message.stories.tsx (1)
118-119: LGTM! Story exports follow the established pattern.The new InfoCard stories are correctly created using the
createStoryhelper and follow the naming conventions used throughout the file.packages/ui-kit/src/blocks/LayoutBlock.ts (1)
8-8: LGTM! InfoCardBlock correctly added to LayoutBlock union.The import and type addition are consistent with the existing pattern and properly extend the LayoutBlock union to support info card layouts.
Also applies to: 25-26
packages/fuselage-ui-kit/src/elements/IconElement.tsx (1)
9-11: LGTM! Simple and correct Icon rendering.The component correctly maps the icon name and variant to the Fuselage Icon component. The fixed size of 20 and color logic are appropriate for this use case.
apps/uikit-playground/src/Payload/actionBlock/BlocksTree.ts (1)
55-55: LGTM! Info card integration follows the established pattern.The new tree node and import are correctly structured and consistent with the existing playground organization.
Also applies to: 235-247
apps/uikit-playground/src/Payload/actionBlock/infoCard/index.ts (2)
13-27: Info-card single-row payload looks correctThe
infoCardPlainpayload matches the expectedinfo_cardblock shape and reuses the icon-button helper appropriately; no issues from a typing or structural perspective.
29-47: Multi-row info-card payload is structurally sound
infoCardMultipleRowscorrectly models a two-row info card with a secondary row; payload structure is consistent with the helper and withLayoutBlock/InfoCardBlockexpectations.packages/fuselage-ui-kit/src/stories/payloads/infoCard.ts (2)
12-26: Single-row info card story payload is well-typed and idiomaticUsing
readonly UiKit.InfoCardBlock[]plusas constgives you strongly typed, immutable story data; layout and action wiring look good.
28-46: Multi-row info card story payload is consistent and reusable
infoCardMultiplemirrors the single-row variant cleanly, with a clear second “duration” row; structure matchesUiKit.InfoCardBlockexpectations.packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (3)
12-20: New imports are consistent with added renderersImporting
InfoCard,IconButtonElement, andIconElementfrom the respective blocks/elements modules matches the new renderer methods below; nothing to adjust here.
65-68: Includinginfo_cardin default allowed blocks is appropriateExtending the default
allowedBlockslist with'info_card'ensures InfoCard blocks render out of the box, consistent with other layout blocks.
357-363:iconrenderer behavior matches other non-interactive elementsSkipping rendering in
BLOCKcontext and delegating toIconElementotherwise is consistent with how other non-interactive elements are handled.
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 14:28", "11/19 15:54 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.14]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.14]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.36]
line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 5 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Wed, 19 Nov 2025 15:54:37 GMT |
… feat/uiKitSectionedPreview
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (2)
349-355: WrapInfoCardinAppIdProviderfor consistency.The
info_cardrenderer does not wrap the component inAppIdProvider, unlike other block-level renderers such asactions,context,divider,section, andcallout. This inconsistency means InfoCard children that depend on app context may not receive it properly.Apply this diff to align with the established pattern:
info_card(block: UiKit.InfoCardBlock, context: UiKit.BlockContext, index: number): ReactElement | null { if (context !== UiKit.BlockContext.BLOCK) { return null; } - return <InfoCard block={block} context={context} index={index} surfaceRenderer={this} />; + return ( + <AppIdProvider key={index} appId={block.appId}> + <InfoCard block={block} context={context} index={index} surfaceRenderer={this} /> + </AppIdProvider> + ); }
365-371: WrapIconButtonElementinAppIdProviderfor consistency.The
icon_buttonrenderer does not wrap the component inAppIdProvider, unlike other interactive element renderers such asbutton,overflow, andstatic_select. Interactive elements require app context for proper scoping of handlers and downstream behavior.Apply this diff to align with the pattern:
icon_button(block: UiKit.IconButtonElement, context: UiKit.BlockContext, index: number): ReactElement | null { if (context === UiKit.BlockContext.BLOCK) { return null; } - return <IconButtonElement block={block} context={context} index={index} surfaceRenderer={this} />; + return ( + <AppIdProvider key={index} appId={block.appId}> + <IconButtonElement block={block} context={context} index={index} surfaceRenderer={this} /> + </AppIdProvider> + ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx(4 hunks)packages/ui-kit/src/blocks/BlockElementType.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (2)
packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
InfoCardBlock(13-16)packages/ui-kit/src/index.ts (4)
InfoCardBlock(46-46)BlockContext(56-56)IconElement(31-31)IconButtonElement(32-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (3)
packages/ui-kit/src/blocks/BlockElementType.ts (1)
29-31: Verification confirms proper integration — code is correct as-is.The enum additions (
ICONandICON_BUTTON) are properly integrated.IconElementandIconButtonElementuse matching string literals ('icon'and'icon_button') that correspond to the enum values, and both types are correctly included in theBlockElementandActionableElementunions.packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (2)
9-9: LGTM: Imports are correct.The new imports for InfoCard, IconButtonElement, and IconElement are properly structured and align with the established conventions in the file.
Also applies to: 19-20
67-67: LGTM: Constructor change is appropriate.Adding 'info_card' to the default allowedBlocks array correctly enables rendering support for the new InfoCard block type.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (2)
349-355: AppIdProvider wrapper is missing for consistency with other block renderers.The
info_cardrenderer doesn't wrap the component inAppIdProvider, unlike other block-level renderers (e.g.,actions,context,section). This inconsistency can break app-scoped behavior if InfoCard children need app context.This was flagged in previous reviews and marked as addressed, but the current code doesn't reflect the fix. Please verify if the wrapper was intentionally omitted or if this is a regression.
357-363: Verify if IconElement requires AppIdProvider.The
iconrenderer doesn't wrapIconElementinAppIdProvider, which was flagged in previous reviews. IfIconElementis non-actionable (display-only likeImageElement), the current implementation may be correct. However, if it needs app context for any reason, it should follow the pattern used by interactive elements.Please confirm whether
IconElementis purely presentational or requires app context.
🧹 Nitpick comments (3)
packages/fuselage-ui-kit/src/blocks/InfoCard.tsx (3)
11-23: Consider making maxWidth configurable.The hardcoded
maxWidth='345px'may limit flexibility for different use cases. If the design system requires a fixed width, this is acceptable; otherwise, consider making it configurable via props or a theme constant.
37-53: Consider stable keys for elements and handle unknown types explicitly.
Key usage: Lines 40 and 47 use
indexas the key. If elements can be reordered or dynamically added/removed, this can cause React reconciliation issues. If the elements have unique identifiers (e.g., an ID or actionId), prefer those over indices.Unknown element types: Line 52 silently returns
nullfor unrecognized element types. Consider adding a development-mode warning to help catch unexpected element types during testing.For unknown types, you could add:
+ if (process.env.NODE_ENV === 'development') { + console.warn(`InfoCard: Unknown element type "${(element as any).type}"`); + } return null;
55-57: UseBoxinstead of<div>for consistency.The component uses
Boxthroughout, but line 55 introduces a bare<div>. For consistency with the rest of the component and to maintain styling flexibility, replace it withBox.Apply this diff:
- <div> + <Box> {action ? <IconButtonElement block={action} context={context} surfaceRenderer={surfaceRenderer} index={index} /> : null} - </div> + </Box>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/fuselage-ui-kit/src/blocks/InfoCard.tsx(1 hunks)packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx(4 hunks)packages/ui-kit/src/blocks/BlockElementType.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (3)
packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
InfoCardBlock(13-16)packages/ui-kit/src/index.ts (4)
InfoCardBlock(46-46)BlockContext(56-56)IconElement(31-31)IconButtonElement(32-32)packages/fuselage-ui-kit/src/contexts/AppIdContext.tsx (1)
AppIdProvider(13-19)
packages/fuselage-ui-kit/src/blocks/InfoCard.tsx (3)
packages/fuselage-ui-kit/src/utils/BlockProps.ts (1)
BlockProps(5-11)packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
InfoCardBlock(13-16)packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (1)
context(113-123)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
packages/ui-kit/src/blocks/BlockElementType.ts (1)
29-31: LGTM!The addition of
ICONandICON_BUTTONenum members is clean and follows the existing pattern. The change is purely additive and non-breaking.packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (3)
9-9: LGTM!The imports are correctly added for the new UI components.
Also applies to: 19-20
67-67: LGTM!The
allowedBlocksarray is correctly updated to include'info_card'.
365-375: LGTM!The
icon_buttonrenderer correctly wrapsIconButtonElementinAppIdProviderand follows the established pattern for interactive elements. The key usesblock.actionIdwith a fallback toindex, which is appropriate.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui-kit/src/surfaces/message/UiKitParserMessage.ts (1)
23-26: Add 'info_card' to the super() block types array.The
InfoCardBlocktype was added to theMessageSurfaceLayoutBlockunion (line 21), but the corresponding block type string'info_card'is missing from the super() constructor call. This inconsistency will prevent InfoCard blocks from being recognized and rendered at runtime.Apply this diff:
export abstract class UiKitParserMessage<OutputElement> extends SurfaceRenderer<OutputElement, MessageSurfaceLayoutBlock> { public constructor() { - super(['actions', 'context', 'divider', 'image', 'section', 'preview', 'video_conf', 'callout']); + super(['actions', 'context', 'divider', 'image', 'section', 'preview', 'video_conf', 'callout', 'info_card']); } }
🧹 Nitpick comments (1)
packages/ui-kit/src/blocks/elements/IconElement.ts (1)
1-7: Consider exportingAvailableIconsfor improved type safety.The
AvailableIconstype is used in the publicIconElementtype but is not itself exported. Exporting it would allow consumers to programmatically reference available icon names with type safety when constructingIconElementobjects or performing validation.Apply this diff:
-type AvailableIcons = 'phone-off' | 'phone-issue' | 'clock' | 'arrow-forward' | 'info'; +export type AvailableIcons = 'phone-off' | 'phone-issue' | 'clock' | 'arrow-forward' | 'info';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/fuselage-ui-kit/src/blocks/InfoCard.tsx(1 hunks)packages/fuselage-ui-kit/src/elements/IconElement.tsx(1 hunks)packages/ui-kit/src/blocks/elements/IconElement.ts(1 hunks)packages/ui-kit/src/blocks/layout/InfoCardBlock.ts(1 hunks)packages/ui-kit/src/surfaces/message/UiKitParserMessage.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/fuselage-ui-kit/src/elements/IconElement.tsx
- packages/ui-kit/src/blocks/layout/InfoCardBlock.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37398
File: packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx:357-363
Timestamp: 2025-11-17T15:07:13.250Z
Learning: In packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx, IconElement is a presentational, non-actionable element that does not require wrapping in AppIdProvider, similar to plain_text and mrkdwn renderers. Only actionable elements (those with actions, actionId, or interactive behavior) should be wrapped in AppIdProvider.
📚 Learning: 2025-11-17T15:07:13.250Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37398
File: packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx:357-363
Timestamp: 2025-11-17T15:07:13.250Z
Learning: In packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx, IconElement is a presentational, non-actionable element that does not require wrapping in AppIdProvider, similar to plain_text and mrkdwn renderers. Only actionable elements (those with actions, actionId, or interactive behavior) should be wrapped in AppIdProvider.
Applied to files:
packages/ui-kit/src/blocks/elements/IconElement.tspackages/fuselage-ui-kit/src/blocks/InfoCard.tsx
🧬 Code graph analysis (3)
packages/ui-kit/src/surfaces/message/UiKitParserMessage.ts (1)
packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
InfoCardBlock(14-17)
packages/ui-kit/src/blocks/elements/IconElement.ts (1)
packages/ui-kit/src/index.ts (1)
IconElement(31-31)
packages/fuselage-ui-kit/src/blocks/InfoCard.tsx (3)
packages/fuselage-ui-kit/src/utils/BlockProps.ts (1)
BlockProps(5-11)packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
InfoCardBlock(14-17)packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (1)
context(113-123)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (1)
361-367: Icon renderer correctly treats IconElement as non-interactive; consider adding a keySkipping
AppIdProviderand only blockingBlockContext.BLOCKis appropriate for the presentationalIconElementand aligns with the pattern used for other non-actionable content (plain text, mrkdwn). Based on learnings.To reduce the chance of React “missing key” warnings and to mirror the
ImageElementbranch, you can optionally add akey:- return <IconElement block={block} context={context} index={index} surfaceRenderer={this} />; + return <IconElement key={index} block={block} context={context} index={index} surfaceRenderer={this} />;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37398
File: packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx:357-363
Timestamp: 2025-11-17T15:07:13.273Z
Learning: In packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx, IconElement is a presentational, non-actionable element that does not require wrapping in AppIdProvider, similar to plain_text and mrkdwn renderers. Only actionable elements (those with actions, actionId, or interactive behavior) should be wrapped in AppIdProvider.
📚 Learning: 2025-11-17T15:07:13.273Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37398
File: packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx:357-363
Timestamp: 2025-11-17T15:07:13.273Z
Learning: In packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx, IconElement is a presentational, non-actionable element that does not require wrapping in AppIdProvider, similar to plain_text and mrkdwn renderers. Only actionable elements (those with actions, actionId, or interactive behavior) should be wrapped in AppIdProvider.
Applied to files:
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx
🧬 Code graph analysis (1)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (4)
packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
InfoCardBlock(14-17)packages/ui-kit/src/index.ts (4)
InfoCardBlock(46-46)BlockContext(56-56)IconElement(31-31)IconButtonElement(32-32)packages/fuselage-ui-kit/src/contexts/AppIdContext.tsx (1)
AppIdProvider(13-19)packages/fuselage-ui-kit/src/stories/Message.stories.tsx (1)
InfoCard(118-118)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (4)
9-20: New imports for InfoCard, IconElement, and IconButtonElement look correctThe added imports follow the existing blocks/elements folder structure and match the new renderer methods below; no issues here.
67-67: Includinginfo_cardin the default allowed blocks is appropriateExtending the default
allowedBlocksarray with'info_card'ensures InfoCard blocks are renderable by default without affecting callers that pass a custom list.
349-359: InfoCard renderer matches existing layout-block/AppIdProvider patternRestricting
info_cardtoBlockContext.BLOCKand wrapping theInfoCardblock inAppIdProviderkeyed byindexandblock.appIdis consistent with other layout blocks (e.g.,section,callout) and correctly propagates app context to any nested actionable elements.
369-379: Icon button renderer is correctly treated as an interactive elementBlocking
BlockContext.BLOCKand wrappingIconButtonElementinAppIdProviderkeyed byblock.actionId || indexmatches how other interactive elements (button, selects, datepicker, etc.) are handled and ensuresappIdis available to downstream handlers. Based on learnings.
gabriellsh
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
tassoevan
changed the title
info_card, icon and icon_button to ui-kit
| }, | ||
| ], | ||
| }, | ||
| ] as const; |
There was a problem hiding this comment.
It's redundant to assert it as const to initialize an already typed variable.
3 participants
Proposed changes (including videos or screenshots)
Example screenshot below
You can also check these components by running
fuselage-ui-kit's storybook, or running theuikit-playground.Issue(s)
VGA-52
Steps to test or reproduce
Further comments
Architectural Decision Record
Summary by CodeRabbit
New Features
Testing / Stories
Chores