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-4080] Shields List + Change main shield #1456

Merged
merged 106 commits into from
Feb 12, 2025
Merged

Conversation

danvleju-rdx
Copy link
Contributor

@danvleju-rdx danvleju-rdx commented Feb 7, 2025

Jira ticket: ABW-4080

Changelog

  • Added shields list screen. Shield status will be implemented in another PR
  • Added change main shield

Video

video

@danvleju-rdx danvleju-rdx marked this pull request as ready for review February 10, 2025 16:31
Copy link
Contributor

@CyonAlexRDX CyonAlexRDX left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@matiasbzurovski matiasbzurovski left a comment

Choose a reason for hiding this comment

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

LGTM, but given radixdlt/sargon#374 is already approved maybe worth waiting for it and remove the TODOs

if case let .selection(isSelected) = mode {
RadioButton(
appearance: .dark,
state: isSelected ? .selected : .unselected
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: we do this in a lot of places, might as well have an init that takes a isSelected: Bool instead

}

// MARK: - ShieldCardStatus
// TODO: define in Sargon ------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

put inside MoveToSargon folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be handled soon, so it’s fine to leave it here for now

@danvleju-rdx danvleju-rdx merged commit caaed68 into main Feb 12, 2025
5 checks passed
@danvleju-rdx danvleju-rdx deleted the dv/ABW-4080-shields-list branch February 12, 2025 09:05
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.

4 participants