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

Wallet creation flow font weight and color tweaks #420

Merged

Conversation

GBKS
Copy link
Contributor

@GBKS GBKS commented Aug 30, 2024

There are some very minor implementation discrepancies from the design in the wallet creation flow. The font weights for several titles are regular, but should be semi-bold (see all the other screens in the onboarding flow for examples). Two divider lines are neutral 7, but should be the neutral 4 color.

See the following image with screenshots for the comparison. Top row shows the before, the bottom row shows the after. The second column is where the two divider line colors were adjusted. The other columns is where the font weight of the header was adjusted.

image

This is my first PR for this project, let me know what I am doing wrong.

The font weights for several titles was regular, but should be semi-bold.
Two divider lines were neutral 7, but should be the neutral 4 color.
Copy link
Contributor

@MarnixCroes MarnixCroes left a comment

Choose a reason for hiding this comment

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

lgtm ACK c753d16

Copy link
Contributor

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

utACK c753d16

In case you have to re-touch the code, usually commit titles have some prefix according to the area of the code is being modified (eg in this case could be "gui: Wallet creation flow font weight and color tweaks"). In the PR's title case, as we are already in the gui (or qml) repo that's not necessary.

@hebasto hebasto merged commit 48bc2c8 into bitcoin-core:main Sep 2, 2024
9 checks passed
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.

4 participants