chore: inline notification visual tweaks #5089
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5089 +/- ##
=======================================
Coverage 85.55% 85.55%
=======================================
Files 724 724
Lines 29479 29486 +7
Branches 5090 5094 +4
=======================================
+ Hits 25221 25227 +6
- Misses 4023 4024 +1
Partials 235 235
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
src/account/Profile.tsx
Outdated
| @@ -110,7 +110,7 @@ function Profile({ navigation, route }: Props) { | |||
| </View> | |||
| <View style={styles.disclaimerContainer}> | |||
| <InLineNotification | |||
| severity={Severity.Informational} | |||
| variant={Variant.Info} | |||
There was a problem hiding this comment.
| variant={Variant.Info} | |
| variant={NotificationVariant.Info} |
maybe? i'm not sure, i wonder if the generic names are confusing. we haven't been terribly consistent with this but the old "Severity" name bothered me. for the button enums, we prefix with "Btn" so it'd be nice to do something similar here?
| <View style={styles.attentionIcon}> | ||
| <Icon color={severityColor.primary} size={20} /> | ||
| </View> | ||
| {icon !== null && ( |
There was a problem hiding this comment.
nit: it seems like we're using icon to either provide a custom icon or hide the default icon, to reduce surprises and implicit meanings in props, could we split this into 2 props? hideIcon and customIcon maybe?
### Description Visual tweaks to accommodate current [design variants](https://www.figma.com/file/erFfzHvSTm5g1sjK6jWyEH/Working-Design-System-Components?type=design&node-id=1144-35&mode=design&t=kUIyzJiXtW1ebQAL-0): * added `Success` variant (green background) * added icon override: can provide alternative icon or `null` to render without one Renames: * `Severity` -> `NotificationVariant` because from my standpoint it's better aligned with all options it contains * `Informational` -> `Info` just because it's shorter ### Test plan * Added unit test for icon override ### Related issues - Related to RET-1004 ### Backwards compatibility Y ### Network scalability NA
Description
Visual tweaks to accommodate current design variants:
Successvariant (green background)nullto render without oneRenames:
Severity->NotificationVariantbecause from my standpoint it's better aligned with all options it containsInformational->Infojust because it's shorterTest plan
Related issues
Backwards compatibility
Y
Network scalability
NA