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

Namadillo: Top Navigation improvements #1475

Merged
merged 2 commits into from
Jan 7, 2025
Merged

Conversation

pedrorezende
Copy link
Collaborator

@pedrorezende pedrorezende commented Jan 6, 2025

  • Adds a modal to help users navigate between options of shielding and unshielding
  • Adds a copy to clipboard to transaction hash

Closes #1409

Copy link
Collaborator

@euharrison euharrison left a comment

Choose a reason for hiding this comment

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

Nice!


{unshieldingModalOpen && (
<UnshieldAssetsModal onClose={() => setUnshieldingModalOpen(false)} />
)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could use the same navigation structure of backgroundLocation because we have other Shield buttons, then it'll be easier to reuse this implementation :)

Copy link
Collaborator Author

@pedrorezende pedrorezende Jan 7, 2025

Choose a reason for hiding this comment

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

I tried to keep it simple for now, do we have any other place that we have such a generic "shield" button that would justify moving this piece to a route?

Copy link
Collaborator

@euharrison euharrison Jan 7, 2025

Choose a reason for hiding this comment

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

The yellow banner on the sidebar

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I remember that it will be removed and re-added when we have the "Shield All" functionality

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, you are right, thanks for reminding it!

We also have this banner on Transparent Overview. But we need to check with @ChrisHoltDesign if we need to update this action as well

Screenshot 2025-01-07 at 15 53 55

Copy link

@ChrisHoltDesign ChrisHoltDesign Jan 7, 2025

Choose a reason for hiding this comment

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

So that 'shield button' in the transparent assets section should ideally load the 'Internal shield option', not the IBC version.

As it's an action to shield assets that are in your Namada transparent account

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks Chris!

So we are good having this modal only on top navigation :)

Choose a reason for hiding this comment

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

Correct :)

@pedrorezende pedrorezende merged commit 741fbec into main Jan 7, 2025
10 checks passed
@pedrorezende pedrorezende deleted the namadillo/ibc-improvements branch January 7, 2025 21:23
@ChrisHoltDesign
Copy link

ChrisHoltDesign commented Jan 8, 2025

@pedrorezende @euharrison This button should also open up the modal

Screenshot 2025-01-08 at 16 36 09

Lets change the text to 'shield assets' also

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.

Proposal to simplify transfer of all assets for end users
3 participants