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

[ABW-3532] Create persona ui adjustments #1059

Merged
merged 11 commits into from
Aug 5, 2024

Conversation

jakub-rdx
Copy link
Contributor

Description

  • updated primary and secondary buttons height and font size according to zeplin
  • updated first time persona creation screen
  • updated persona creation screen

Screenshot

Before After

…stments

# Conflicts:
#	app/src/main/java/com/babylon/wallet/android/presentation/settings/personas/createpersona/CreatePersonaInfoScreen.kt
#	app/src/main/java/com/babylon/wallet/android/presentation/settings/personas/createpersona/CreatePersonaScreen.kt
…stments

# Conflicts:
#	designsystem/src/main/java/com/babylon/wallet/android/designsystem/composable/RadixPrimaryButton.kt
#	designsystem/src/main/java/com/babylon/wallet/android/designsystem/composable/RadixSecondaryButton.kt
Copy link
Contributor

@sergiupuhalschi-rdx sergiupuhalschi-rdx 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 minor observations regarding CreatePersonaScreen:

  • The top padding seems too big (between the screen top and the header) compared to zeplin
  • The padding between the avatar placeholder and “Persona label” seems smaller compared to zeplin
  • The padding between the divider and the “Some dApps may request..” text seems smaller compared to zeplin
  • Persona Label input hint should have gray color
  • It would be nice to set sentences capitalization to the label input

…stments

# Conflicts:
#	designsystem/src/main/java/com/babylon/wallet/android/designsystem/composable/RadixSecondaryButton.kt
Copy link
Contributor

Choose a reason for hiding this comment

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

The persona label currently supports multiple lines, but it should be single line and with Done ime action. Also maybe it should have KeyboardCapitalization.Words since it's a name? Sorry for mentioning "Sentences" in the previous review.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jakub-rdx don't forget about this. IMHO this will bring much value with a little effort.

@micbakos-rdx
Copy link
Contributor

Besides on the points @sergiupuhalschi-rdx mentioned I also noticed that in the confirmation screen the bottom button has bigger paddings than than the button in the creation flow. Also should this button be a bottom bar and use a divider? Take a look at the video:

selection.webm

@GhenadieVP
Copy link
Contributor

On Congratulations screen the CTA should have a "Continue to" prefix

@jakub-rdx
Copy link
Contributor Author

On Congratulations screen the CTA should have a "Continue to" prefix

already updated

@jakub-rdx
Copy link
Contributor Author

Besides on the points @sergiupuhalschi-rdx mentioned I also noticed that in the confirmation screen the bottom button has bigger paddings than than the button in the creation flow. Also should this button be a bottom bar and use a divider? Take a look at the video:

selection.webm

updated with RadixBotomBar

…stments

# Conflicts:
#	app/src/main/java/com/babylon/wallet/android/presentation/settings/personas/createpersona/CreatePersonaScreen.kt
#	designsystem/src/main/java/com/babylon/wallet/android/designsystem/composable/RadixSecondaryButton.kt
@jakub-rdx jakub-rdx merged commit 09cd9d5 into main Aug 5, 2024
6 checks passed
@jakub-rdx jakub-rdx deleted the feature/ABW-3532-create-persona-flow-ui-adjustments branch August 5, 2024 09:28
Copy link

sonarcloud bot commented Aug 5, 2024

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

Successfully merging this pull request may close these issues.

5 participants