-
Notifications
You must be signed in to change notification settings - Fork 160
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
Compound: add BigIcon
, BigCheckmark
and PageTitle
components
#2574
Conversation
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2574 +/- ##
===========================================
+ Coverage 73.16% 73.19% +0.02%
===========================================
Files 1410 1413 +3
Lines 34202 34291 +89
Branches 6638 6670 +32
===========================================
+ Hits 25024 25098 +74
- Misses 5713 5718 +5
- Partials 3465 3475 +10 ☔ 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.
LGTM, thanks!
) { | ||
val backgroundColor = when (style) { | ||
is Style.Default -> ElementTheme.colors.bigIconDefaultBackgroundColor | ||
Style.AlertSolid, Style.SuccessSolid -> ElementTheme.colors.bgCanvasDefault |
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.
I am wondering if using a transparent color should be preferable in this case. I let you decide.
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.
Transparent is probably better, although it kind of contradicts its name. I'm asking in the Compound room to clarify this.
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.
I just confirmed it with them, it's the other way around: the solid ones should have a solid background (duh), the others should be transparent. I'll change those.
internal class BigIconStylePreviewProvider : PreviewParameterProvider<BigIcon.Style> { | ||
override val values: Sequence<BigIcon.Style> | ||
get() = sequenceOf( | ||
BigIcon.Style.Default(Icons.Filled.CatchingPokemon), |
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.
:)
Make the non-solid ones use a transparent one.
Quality Gate passedIssues Measures |
Type of change
Content
Adds 3 new Compound components that will be needed for future development:
Motivation and context
These components will be used in new designs and will replace older ones we were using.
Screenshots / GIFs
Being recorded.
Tests
Nothing, but you can check the screenshots look like the designs in Figma.
Tested devices
Checklist