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

chore(suite): refactor staking #15391

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

adamhavel
Copy link
Contributor

@adamhavel adamhavel commented Nov 14, 2024

  • refactored staking dashboard and claim modal
  • added flex to frame props
  • added paddingType to IconCircle component
  • added iconName to InfoRow component
  • removed hasBottomPadding from form components
  • fixed prop passing in form components

Resolves #15398

Screenshots:

Before

Screenshot 2024-11-14 at 14 11 24 Screenshot 2024-11-14 at 14 11 38

After

Screenshot 2024-11-14 at 13 57 35 Screenshot 2024-11-14 at 13 58 18

@adamhavel adamhavel self-assigned this Nov 14, 2024
@adamhavel adamhavel marked this pull request as ready for review November 14, 2024 13:15
@adamhavel adamhavel requested review from jvaclavik and removed request for jvaclavik November 14, 2024 13:15
@adamhavel adamhavel force-pushed the chore/refactor-staking branch 4 times, most recently from 40b0d77 to ff95364 Compare November 14, 2024 21:09
Copy link
Member

@tomasklim tomasklim left a comment

Choose a reason for hiding this comment

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

Claiming is usually not available, so most of the users will see it like this
Screenshot 2024-11-15 at 8 51 31

This banner looks weird in the bottom
At the top or better at the modal header as description, it would maybe look better?
Screenshot 2024-11-15 at 8 47 40

Probably you removed it in the previous refactoring but the fee component had some sort of state that the previous gas limit and gas price was visible before the new one was composed. That was there to avoid this jumping

Screen.Recording.2024-11-15.at.8.48.39.mov

@adamhavel
Copy link
Contributor Author

adamhavel commented Nov 15, 2024

Claiming is usually not available, so most of the users will see it like this

Ok, I didn't know that, I thought there is always either claiming available or pending:) I'll fix that.

@tomasklim
Copy link
Member

/rebase

Copy link

@tomasklim
Copy link
Member

@trezor/qa this PR should be just staking UI/UX changes, but please test it properly 🙏

Copy link
Member

@tomasklim tomasklim left a comment

Choose a reason for hiding this comment

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

LGTM

Would be nice to find a solution for this claiming banner as it is huge but I do not want to block merging to avoid conflicts
Screenshot 2024-11-15 at 17 54 00

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

Successfully merging this pull request may close these issues.

Unused input props drilled to DOM
2 participants