BREAKING_CHANGE: update DotIndicator sizes#727
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR updates the DotIndicator size API across React and React Native and refactors Avatar notification dots to use DotIndicator, including suppressing notification dots for larger avatar sizes.
Changes:
- Replaces the
DotIndicatorsize scale fromxs/sm/md/lgtosm/md/lg/xland changes the default tomd. - Refactors
Avatarnotification rendering to useDotIndicatorand only render onsm/mdavatars. - Updates stories, MDX docs, sandbox usage, and adds version plans for both packages.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
.nx/version-plans/version-plan-1780069206903-rnative.md |
Adds rnative release/migration notes for the breaking size change. |
.nx/version-plans/version-plan-1780069206903-react.md |
Adds React release/migration notes for the breaking size change. |
apps/app-sandbox-rnative/src/app/blocks/DotIndicators.tsx |
Updates sandbox examples to the new DotIndicator size scale. |
libs/ui-rnative/src/lib/Components/Avatar/Avatar.mdx |
Documents Avatar notification suppression for larger sizes. |
libs/ui-rnative/src/lib/Components/Avatar/Avatar.tsx |
Uses DotIndicator for Avatar notifications and gates rendering by size. |
libs/ui-rnative/src/lib/Components/DotIndicator/DotIndicator.mdx |
Updates rnative DotIndicator size documentation. |
libs/ui-rnative/src/lib/Components/DotIndicator/DotIndicator.stories.tsx |
Updates rnative DotIndicator story controls/examples. |
libs/ui-rnative/src/lib/Components/DotIndicator/DotIndicator.test.tsx |
Updates rnative DotIndicator size render tests. |
libs/ui-rnative/src/lib/Components/DotIndicator/DotIndicator.tsx |
Implements the new rnative DotIndicator size mapping/default. |
libs/ui-rnative/src/lib/Components/DotIndicator/types.ts |
Updates rnative DotIndicator public size type/default docs. |
libs/ui-react/src/lib/Components/Avatar/Avatar.mdx |
Documents Avatar notification suppression for larger sizes. |
libs/ui-react/src/lib/Components/Avatar/Avatar.tsx |
Uses DotIndicator for Avatar notifications and gates rendering by size. |
libs/ui-react/src/lib/Components/DotIndicator/DotIndicator.mdx |
Updates React DotIndicator size documentation. |
libs/ui-react/src/lib/Components/DotIndicator/DotIndicator.stories.tsx |
Updates React DotIndicator story controls/examples. |
libs/ui-react/src/lib/Components/DotIndicator/DotIndicator.test.tsx |
Updates React DotIndicator size render tests. |
libs/ui-react/src/lib/Components/DotIndicator/DotIndicator.tsx |
Implements the new React DotIndicator size mapping/default. |
libs/ui-react/src/lib/Components/DotIndicator/types.ts |
Updates React DotIndicator public size type/default docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // dot indicator is not visible on larger sizes, regardless of the `showNotification` prop | ||
| const showNotification = | ||
| showNotificationProp && (size === 'sm' || size === 'md'); | ||
|
|
There was a problem hiding this comment.
I'm not sure about this rule. I understand that we don't want consumers to add badges when the avatar size is large, but I don't think enforcing that is the Avatar component's responsibility. In my opinion, it's up to consumers to use the component correctly and follow the intended usage in Figma.
so maybe we should let the consumers add the dot themselves as we do on other components ?
There was a problem hiding this comment.
based on discussion on the daily, created a ticket:
https://ledgerhq.atlassian.net/browse/DLS-770
zel-kass
left a comment
zel-kass
left a comment
There was a problem hiding this comment.
Very clean work as always ! Pre-approving.
I just have one concern on Avatar, I was wondering about the fallback icon. Right now, in dark mode, it is barely visible because we have set a black icon :
On Figma, a white Icon is used for both but it is only because it sits on a black background, that is something that we don't have, so I see two options to improve that.
- We conditionaly display a white Icon depending on the theme to have something like this in dark mode :
- We align with Figma and put a dark background :
Although this is a linear gradient, so we might need to sync with Laurine. This is just an open question btw, maybe I missed something, and it doesn't have anything to do with the code in this PR. 🫰🏽
|
|
@zel-kass nice catch, thank you 🕵️ |
Closes DLS-757.
Considerations
md(fromsm) with a pixel size of12px(from8px).Avatarto useDotIndicator, syncing the logic between both libs.sm(lgdot) andmd(xldot). For larger avatar sizeslgandxl, even if theshowNotificationprop is present, the badge will not show. See demo below:Demo
Tested on iOS 26.5 (iPhone 17 simulator).