feat: Add framed optional property for IconElement ui-kit component
#37726
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 100d9ad 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 a frameable icon variation: introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
📦 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 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 14:05 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 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, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 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, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 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.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 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.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 18 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Fri, 12 Dec 2025 14:05:53 GMT |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37726 +/- ##
========================================
Coverage 67.71% 67.72%
========================================
Files 3452 3458 +6
Lines 113979 113955 -24
Branches 20943 20927 -16
========================================
- Hits 77182 77173 -9
+ Misses 34663 34651 -12
+ Partials 2134 2131 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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 (8)
.changeset/many-walls-cheat.md(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/infoCard.ts(2 hunks)packages/ui-kit/src/blocks/BlockElement.ts(2 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(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/fuselage-ui-kit/src/stories/Message.stories.tsxpackages/ui-kit/src/blocks/elements/IconElement.tspackages/ui-kit/src/blocks/BlockElement.tspackages/ui-kit/src/index.tspackages/fuselage-ui-kit/src/stories/payloads/infoCard.tspackages/ui-kit/src/blocks/layout/InfoCardBlock.tspackages/fuselage-ui-kit/src/elements/IconElement.tsx
🧠 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/ui-kit/src/blocks/elements/IconElement.tspackages/ui-kit/src/blocks/BlockElement.tspackages/ui-kit/src/index.tspackages/ui-kit/src/blocks/layout/InfoCardBlock.tspackages/fuselage-ui-kit/src/elements/IconElement.tsx.changeset/many-walls-cheat.md
🧬 Code graph analysis (5)
packages/ui-kit/src/blocks/elements/IconElement.ts (1)
packages/ui-kit/src/index.ts (2)
IconElement(31-31)FrameableIconElement(31-31)
packages/ui-kit/src/blocks/BlockElement.ts (2)
packages/ui-kit/src/blocks/elements/IconElement.ts (1)
FrameableIconElement(9-11)packages/ui-kit/src/index.ts (1)
FrameableIconElement(31-31)
packages/fuselage-ui-kit/src/stories/payloads/infoCard.ts (2)
packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
InfoCardBlock(14-17)packages/ui-kit/src/index.ts (1)
InfoCardBlock(46-46)
packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (2)
packages/ui-kit/src/blocks/elements/IconElement.ts (1)
FrameableIconElement(9-11)packages/ui-kit/src/index.ts (3)
FrameableIconElement(31-31)PlainText(5-5)Markdown(6-6)
packages/fuselage-ui-kit/src/elements/IconElement.tsx (3)
packages/ui-kit/src/blocks/elements/IconElement.ts (2)
FrameableIconElement(9-11)IconElement(3-7)packages/ui-kit/src/index.ts (2)
FrameableIconElement(31-31)IconElement(31-31)packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (1)
icon(361-367)
🪛 LanguageTool
.changeset/many-walls-cheat.md
[grammar] ~6-~6: Ensure spelling is correct
Context: ...at/ui-kit": minor --- Introduces a new varation of the Icon element to ui-kit throu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (8)
packages/ui-kit/src/blocks/elements/IconElement.ts (1)
1-11: LGTM!The type definitions are well-structured. The expansion of the
variantunion to include'warning'and the introduction ofFrameableIconElementas an extension ofIconElementwith an optionalframedproperty follow a clean composition pattern.packages/fuselage-ui-kit/src/stories/Message.stories.tsx (1)
120-120: LGTM!The new story export follows the established pattern and properly demonstrates the framed icon functionality.
packages/ui-kit/src/index.ts (1)
31-31: LGTM!The public API export correctly includes both
IconElementandFrameableIconElement, enabling consumers to use either type as appropriate.packages/ui-kit/src/blocks/BlockElement.ts (1)
8-8: LGTM!The migration from
IconElementtoFrameableIconElementin theBlockElementunion is consistent with the broader type evolution and ensures the framed property is available throughout the type system.Also applies to: 43-43
packages/fuselage-ui-kit/src/stories/payloads/infoCard.ts (2)
35-35: LGTM!The variant change to
'warning'demonstrates the newly added warning variant capability.
48-73: LGTM!The new
infoCardMultipleIconspayload effectively demonstrates both framed and non-framed icon variants across all supported variant types (default, warning, danger, secondary), providing comprehensive visual coverage of the new feature.packages/ui-kit/src/blocks/layout/InfoCardBlock.ts (1)
4-4: LGTM!The type update from
IconElementtoFrameableIconElementinInfoCardRow.elementsis consistent with the broader refactor and enables framed icon support in info card blocks.Also applies to: 10-10
packages/fuselage-ui-kit/src/elements/IconElement.tsx (1)
1-3: LGTM!The implementation cleanly handles both framed and non-framed icon rendering. The helper functions
getVariantColorandgetFramedIconPropsprovide clear mapping logic for variant properties, and the conditional rendering based on theframedproperty is straightforward and correct.Also applies to: 7-36, 38-39, 41-45
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
d-gubert
force-pushed
the
feat/uiKitFramedIcon
branch
from
c04f05a to
069ef98
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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)
.changeset/many-walls-cheat.md(1 hunks)packages/fuselage-ui-kit/src/elements/IconElement.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/many-walls-cheat.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/fuselage-ui-kit/src/elements/IconElement.tsx
🧠 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/elements/IconElement.tsx
⏰ 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/elements/IconElement.tsx (4)
1-3: LGTM: Imports are appropriate for framed icon support.The additions of
FramedIconandComponentPropsare necessary for the new framed icon functionality.
7-7: LGTM: Type updated to support framed icons.The change from
IconElementtoFrameableIconElementcorrectly reflects the new optionalframedproperty.
9-21: The'status-font-on-warning'color token is valid and widely used throughout the codebase for warning status indicators. No changes needed.
23-36: No issues identified. ThegetFramedIconPropsfunction correctly maps variants to FramedIcon props using type-safe TypeScript patterns withComponentPropsandPick, ensuringdanger,warning, andneutralprops exist on the FramedIcon component.
tassoevan
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/fuselage-ui-kit/src/elements/IconElement.tsx (1)
23-36: Consider a more precise return type for the variant-to-props mapping.The variant mapping logic is correct and complete. However, the return type could be more precise. Currently,
Pick<ComponentProps<typeof FramedIcon>, 'warning' | 'danger' | 'neutral'>creates optional properties, whereas the function always returns exactly one property set totrue.For better type safety, use a union type:
const getFramedIconProps = ( variant: UiKit.FrameableIconElement['variant'], -): Pick<ComponentProps<typeof FramedIcon>, 'warning' | 'danger' | 'neutral'> => { +): { warning: true } | { danger: true } | { neutral: true } => {This makes the type system reflect the actual runtime behavior more accurately.
📜 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)
.changeset/many-walls-cheat.md(1 hunks)packages/fuselage-ui-kit/src/elements/IconElement.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/many-walls-cheat.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/fuselage-ui-kit/src/elements/IconElement.tsx
🧠 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/elements/IconElement.tsx
⏰ 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/elements/IconElement.tsx (4)
1-3: LGTM! Imports are clean and necessary.The added
FramedIconandComponentPropsimports are used appropriately in the implementation.
7-7: Type change aligns with the new framed icon capability.The transition to
FrameableIconElementcorrectly supports the newframedproperty.
9-21: Variant color mapping is comprehensive and correct.The addition of the
warningvariant and explicitdefaultcase properly handles all FrameableIconElement variants for non-framed icon rendering.
38-44: Rendering logic cleanly implements the framed/non-framed behavior.The conditional rendering correctly switches between
FramedIcon(when framed) andIcon(when not framed), with appropriate prop mapping for each case.
…nt (#37726) Co-authored-by: Douglas Gubert <1810309+d-gubert@users.noreply.github.com>
4 participants
Proposed changes (including videos or screenshots)
Issue(s)
VGA-89
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.