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

Ui tweaks 1 #2601

Merged
merged 11 commits into from
Dec 9, 2024
Merged

Ui tweaks 1 #2601

merged 11 commits into from
Dec 9, 2024

Conversation

Copy link

netlify bot commented Dec 6, 2024

Deploy Preview for decent-interface-dev ready!

Name Link
🔨 Latest commit 988d258
🔍 Latest deploy log https://app.netlify.com/sites/decent-interface-dev/deploys/67544e0c20844f0008d2b741
😎 Deploy Preview https://deploy-preview-2601.app.dev.decentdao.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@mudrila mudrila left a comment

Choose a reason for hiding this comment

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

Couple minor comments but otherwise - looks good!

src/components/ui/menus/SafesMenu/index.tsx Outdated Show resolved Hide resolved
src/components/ui/menus/SafesMenu/index.tsx Outdated Show resolved Hide resolved
Copy link

@nicolaus-sherrill nicolaus-sherrill left a comment

Choose a reason for hiding this comment

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

Within "Governance Parameters"
Replace the color blocks around the info with a Neutral 3 outline. Current color blocks are very low contrast and don't need to have a separate fill.
( we fill surfaces to indicate interactivity and outline content that is readable but not interactive)

Also please reduce the gap between both blocks to 0.5rem

Leave "Add Permissions" button up top.

@nicolaus-sherrill
Copy link

When I click on "My DAOs" with an empty list, a 1px width div appears but the empty state of the menu doesn't appear

Screenshot 2024-12-06 at 10 57 46 AM

@parkermccurley
Copy link

parkermccurley commented Dec 6, 2024

Edit icon / active hover state shouldn't be visible in Permissions if I don't have the ability to edit

Screenshot 2024-12-06 at 14 37 53

@DarksightKellar
Copy link
Contributor Author

DarksightKellar commented Dec 7, 2024

Within "Governance Parameters" Replace the color blocks around the info with a Neutral 3 outline. Current color blocks are very low contrast and don't need to have a separate fill. ( we fill surfaces to indicate interactivity and outline content that is readable but not interactive)

Also please reduce the gap between both blocks to 0.5rem

Leave "Add Permissions" button up top.

@nicolaus-sherrill
This is a 0.5rem gap. Doesn't look good to me
Screenshot 2024-12-07 at 12 33 46

1.5rem:
Screenshot 2024-12-07 at 12 36 40

2.5rem:
Screenshot 2024-12-07 at 12 37 51

EDIT: Gonna go with 1.5rem until pushback if any

Also the "Add Permissions" being at the bottom was an ask in the PRD (the comment is gone now but @parkermccurley can you confirm)

@DarksightKellar DarksightKellar merged commit 9bb71ce into develop Dec 9, 2024
7 checks passed
@DarksightKellar DarksightKellar deleted the ui-tweaks-1 branch December 9, 2024 14:20
@nicolaus-sherrill
Copy link

nicolaus-sherrill commented Dec 9, 2024

@DarksightKellar With the titles contained within the card there, 0.5 rem would've been the call.
Now that you've moved those titles outside of the cards, 1.5rem is the right call.

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.

5 participants