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.

Sign up for GitHub

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

Jump to bottom

[VDG] Tile system refactoring #9493

Merged
merged 17 commits into from Nov 18, 2022
Merged

[VDG] Tile system refactoring #9493

merged 17 commits into from Nov 18, 2022

Conversation

SuperJMN
Copy link
Collaborator

@SuperJMN SuperJMN commented Nov 7, 2022 •
edited

Refactor of the Tile system to be simpler = more maintainable.

There are no different layouts (normal, small, large).

PR #9435 will be way easier with these changes.

@SuperJMN SuperJMN changed the title Tile system refactoring [VDG] Tile system refactoring Nov 7, 2022
@SuperJMN SuperJMN marked this pull request as ready for review November 7, 2022 11:38
@SuperJMN SuperJMN requested a review from soosr November 7, 2022 11:55
yahiheb
yahiheb reviewed Nov 7, 2022
View reviewed changes
Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

Tested this and everything seems to work fine.

WalletWasabi.Fluent/ViewModels/Wallets/WalletViewModel.cs Outdated Show resolved Hide resolved
@SuperJMN @yahiheb
Update WalletWasabi.Fluent/ViewModels/Wallets/WalletViewModel.cs
e53a6c5 
Co-authored-by: yahiheb <52379387+yahiheb@users.noreply.github.com>
jmacato
jmacato previously approved these changes Nov 10, 2022
View reviewed changes
Copy link
Contributor

@jmacato jmacato left a comment

Choose a reason for hiding this comment

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

Spring cleaning time! In winter lol

soosr
soosr reviewed Nov 10, 2022
View reviewed changes
Copy link
Collaborator

@soosr soosr left a comment

Choose a reason for hiding this comment

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

Very good job, only nits for consistency:

WalletWasabi.Fluent/Views/Wallets/Home/Tiles/BtcPriceTileView.axaml Outdated Show resolved Hide resolved
WalletWasabi.Fluent/Views/Wallets/Home/Tiles/CommonStyles.axaml Outdated Show resolved Hide resolved
WalletWasabi.Fluent/Views/Wallets/Home/Tiles/WalletBalanceTileView.axaml Outdated Show resolved Hide resolved
WalletWasabi.Fluent/Views/Wallets/Home/TilesView.axaml Outdated Show resolved Hide resolved
@SuperJMN SuperJMN requested a review from soosr November 11, 2022 15:59
yahiheb
yahiheb previously approved these changes Nov 13, 2022
View reviewed changes
Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

Tested, LGTM.

MaxHillebrand
MaxHillebrand previously approved these changes Nov 14, 2022
View reviewed changes
Copy link
Member

@MaxHillebrand MaxHillebrand left a comment

Choose a reason for hiding this comment

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

tACK 57997df

@SuperJMN SuperJMN dismissed stale reviews from MaxHillebrand and yahiheb via c9b1369 November 14, 2022 15:23
yahiheb
yahiheb previously approved these changes Nov 15, 2022
View reviewed changes
Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

tACK

soosr
soosr requested changes Nov 15, 2022
View reviewed changes
Copy link
Collaborator

@soosr soosr left a comment

Choose a reason for hiding this comment

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

Empty wallet, privacy tile is broken.
image

WalletWasabi.Fluent/Views/Wallets/Home/TilesView.axaml Outdated Show resolved Hide resolved
@SuperJMN
Copy link
Collaborator Author

Empty wallet, privacy tile is broken. image

Good catch! Fixed. Thank you :)

soosr
soosr requested changes Nov 16, 2022
View reviewed changes
Copy link
Collaborator

@soosr soosr left a comment

Choose a reason for hiding this comment

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

  • The position of the percentage is broken. (Please always test)
  • The gap above and below the Private amount is different, it gives a weird feeling when I look at it. Probably my aesthetic personality screaming inside.

Current:
image

Master:
image

@SuperJMN SuperJMN requested a review from soosr November 16, 2022 15:15
yahiheb
yahiheb previously approved these changes Nov 16, 2022
View reviewed changes
Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

Tested again, the tiles look good to me.

@SuperJMN SuperJMN requested a review from soosr November 18, 2022 10:04
soosr
soosr approved these changes Nov 18, 2022
View reviewed changes
Copy link
Collaborator

@soosr soosr left a comment

Choose a reason for hiding this comment

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

tACK

@soosr soosr merged commit 710f4b9 into zkSNACKs:master Nov 18, 2022
@jmacato jmacato deleted the tiles-refactor branch November 18, 2022 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soosr soosr soosr approved these changes

@jmacato jmacato jmacato left review comments

@MaxHillebrand MaxHillebrand MaxHillebrand left review comments

@yahiheb yahiheb yahiheb left review comments

@nopara74 nopara74 Awaiting requested review from nopara74

@wieslawsoltes wieslawsoltes Awaiting requested review from wieslawsoltes

Assignees
No one assigned
Labels
size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@SuperJMN @wieslawsoltes @soosr @jmacato @MaxHillebrand @yahiheb