-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
design : CounterAtom #4108
design : CounterAtom #4108
Conversation
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Quality Gate passedIssues Measures |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4108 +/- ##
========================================
Coverage 83.27% 83.28%
========================================
Files 1884 1885 +1
Lines 49068 49098 +30
Branches 5760 5764 +4
========================================
+ Hits 40862 40890 +28
Misses 6139 6139
- Partials 2067 2069 +2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, just a small remark about a color!
modifier = Modifier.align(Alignment.Center), | ||
text = countAsText, | ||
style = textStyle, | ||
color = Color.White, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to Figma, you should use ElementTheme.colors.textOnSolidPrimary
(which is White so will not touch the screenshot)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not exactly true, it's using Light/color/text/on-solid/primary
for both Light and Dark theme, so I think it's clearer to use White directly...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yes ok, so ignore me
Content
Introduce new CounterAtom component and ListItemContent.Counter and use it for knock requests.
Motivation and context
https://www.figma.com/design/G1xy0HDZKJf5TCRFmKb5d5/Compound-Android-Components?node-id=2805-2649&m=dev
https://www.figma.com/design/7TqjqdMBaPpm3IavKsMYu6/Ask-to-join-(Knocking)?node-id=5135-92555&m=dev
Screenshots / GIFs
Check new screenshots.
Tests
Tested devices
Checklist