fix(colors): explicitly define colors in places that was missing them #6426
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6426 +/- ##
=======================================
Coverage 89.12% 89.12%
=======================================
Files 735 735
Lines 31858 31859 +1
Branches 6054 6054
=======================================
+ Hits 28392 28395 +3
+ Misses 3271 3269 -2
Partials 195 195
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
We should add |
| @@ -271,6 +271,7 @@ function TokenBottomSheet({ | |||
| keyExtractor={(item) => item.tokenId} | |||
| contentContainerStyle={{ | |||
| paddingBottom: insets.bottom, | |||
| backgroundColor: Colors.backgroundPrimary, | |||
There was a problem hiding this comment.
guess adding one here doesn't hurt anything, but is this required if we specify one in the Navigator?
There was a problem hiding this comment.
oh yes this is needed for when this TokenBottomSheet is used as a normal component (the swap screen)
src/navigator/Navigator.tsx
Outdated
| @@ -757,6 +758,7 @@ function RootStackScreen() { | |||
| // take up the whole screen, it is no longer obvious that they are a bottom | |||
| // sheet / how to navigate away | |||
| maxDynamicContentSize: variables.height * 0.9, | |||
| backgroundStyle: { backgroundColor: Colors.backgroundPrimary }, | |||
There was a problem hiding this comment.
should we also add the handleIndicatorStyle here similar to BottomSheetBase so that it is consistent?
There was a problem hiding this comment.
oooooh yes i missed this!!
@MuckT i'll work with Kayla to decide on a color here - |
…llet into kathy/fix-undefined-colors
| @@ -83,6 +83,7 @@ export class CTextInput extends React.Component<Props, State> { | |||
| inputStyle, | |||
| ]} | |||
| value={value} | |||
| selectionColor={Colors.contentSecondary} | |||
There was a problem hiding this comment.
i'm going with a slightly softer color here for now, and will check with Kayla / update it following our working session later today
There was a problem hiding this comment.
update on this @MuckT after Kayla and I caught up we landed on contentSecondary because it's a little softer in light mode so i'll leave this here :)
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Description
When playing around with changing the colors to another app (Beefy) I noticed there were components that default to black / white that don't work well for a darker theme. This PR fixes those places to make sure the correct color is applied.
Test plan
n/a
Related issues
Backwards compatibility
Y
Network scalability
If a new NetworkId and/or Network are added in the future, the changes in this PR will: