Skip to content
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

US-2150: Developers need an easy way to change the colors of the wallet #908

Open
wants to merge 53 commits into
base: develop
Choose a base branch
from

Conversation

rodrigoncalves
Copy link
Collaborator

No description provided.

@rodrigoncalves rodrigoncalves self-assigned this Mar 25, 2024
@rodrigoncalves rodrigoncalves changed the title Us 2150 US-2150: Developers need an easy way to change the colors of the wallet Mar 25, 2024
@rodrigoncalves rodrigoncalves marked this pull request as ready for review March 28, 2024 03:30
Copy link
Collaborator

@TravellerOnTheRun TravellerOnTheRun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things still need to be solved. And as a suggestion for testing purposes, create a .env variable to switch colors for testing purposes something like theme: "dark" | "light". And choosing this when exporting sharedColors and assign one of two objects. This wasy @jormelCoin can test the actual behaviour and make sure the styles update correctly all over the app.

@@ -86,7 +86,7 @@
"react-native-screens": "^3.29.0",
"react-native-screenshot-prevent": "^1.1.9",
"react-native-ssl-public-key-pinning": "^1.1.3",
"react-native-svg": "^14.1.0",
"react-native-svg": "13.4.0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait! Why downgrading?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to downgrade this version to fix an issue with the appearance of the loading screen using a light background.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rodrigoncalves can you comment or add screenshot/link to show what issue is being caused by downgrading this library?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/components/BasicRow/BasicRowWithContact.tsx Outdated Show resolved Hide resolved
src/components/button/index.tsx Outdated Show resolved Hide resolved
src/components/icons/DeployWalletImage.tsx Outdated Show resolved Hide resolved
src/components/icons/HomeIcon.tsx Outdated Show resolved Hide resolved
<Typography
type={'body3'}
style={styles.footerText}
color={sharedColors.white}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it supposed to be like shared.text.primary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is the onboard screen. This screen should be the same for all diferent themes because we're using an image in background.

src/screens/settings/RelayDeployScreen.tsx Outdated Show resolved Hide resolved
@@ -25,6 +24,7 @@ import { castStyle } from 'shared/utils'
import { changeTopColor } from 'store/slices/settingsSlice'
import { useAppDispatch } from 'store/storeUtils'
import { WalletContext } from 'shared/wallet'
import NoDappsImage from 'components/icons/NoDappsImage'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

named export

src/ux/appFooter/index.tsx Outdated Show resolved Hide resolved
version "14.1.0"
resolved "https://registry.yarnpkg.com/react-native-svg/-/react-native-svg-14.1.0.tgz#7903bddd3c71bf3a8a503918253c839e6edaa724"
integrity sha512-HeseElmEk+AXGwFZl3h56s0LtYD9HyGdrpg8yd9QM26X+d7kjETrRQ9vCjtxuT5dCZEIQ5uggU1dQhzasnsCWA==
react-native-svg@13.4.0:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, keep newest version

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was necessary.

src/components/icons/NoDappsImage.tsx Outdated Show resolved Hide resolved
src/ux/appFooter/index.tsx Outdated Show resolved Hide resolved
@jormelCoin
Copy link
Contributor

A few things still need to be solved. And as a suggestion for testing purposes, create a .env variable to switch colors for testing purposes something like theme: "dark" | "light". And choosing this when exporting sharedColors and assign one of two objects. This wasy @jormelCoin can test the actual behaviour and make sure the styles update correctly all over the app.

Hey @TravellerOnTheRun, I set the var in the .env as you mentioned, but when I put the light option, I didn't see any difference. Please let me know if I put it correctly.
Screenshot 2024-04-17 at 22 06 03

On the other side, I called my attention to the white border on some buttons. I'm conscious that it's a minor issue, but I don't know I see a little different. were these modified intentionally?

Screenshot 2024-04-17 at 21 14 38 Screenshot 2024-04-17 at 21 50 07

@rodrigoncalves
Copy link
Collaborator Author

Hey @TravellerOnTheRun, I set the var in the .env as you mentioned, but when I put the light option, I didn't see any difference. Please let me know if I put it correctly.

This has not yet been implemented.

@jormelCoin
Copy link
Contributor

Hey @TravellerOnTheRun, I set the var in the .env as you mentioned, but when I put the light option, I didn't see any difference. Please let me know if I put it correctly.

This has not yet been implemented.

I assume that will be implemented in another ticket? if so, I think this ticket is working well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants