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

Hover, selected states for App navigation #2063

Merged
merged 8 commits into from
Apr 1, 2022
Merged

Conversation

tanmoyAtb
Copy link
Contributor

@tanmoyAtb tanmoyAtb commented Mar 31, 2022

closes #1678

This issue is quite an old one. I made the changes in point 1,
However the requested changes in 2, 3 didn't feel like it was improving UX, I left them out.
Let me know what you think if 2 or 3 are needed.


Submission checklist:

Layout

  • Change looks good in the desktop web ui
  • Change looks good in the mobile web ui

Theme

  • Components / elements inspected in light mode
  • Components / elements inspected in dark mode

Billing

  • Change applied to both files-ui and storage-ui

@render
Copy link

render bot commented Mar 31, 2022

@render
Copy link

render bot commented Mar 31, 2022

@github-actions github-actions bot added the Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes. label Mar 31, 2022
@render
Copy link

render bot commented Mar 31, 2022

@tanmoyAtb tanmoyAtb marked this pull request as ready for review March 31, 2022 19:15
@tanmoyAtb tanmoyAtb changed the title Mnt/focus hover styles 1678 Hover, selected states for App navigation Mar 31, 2022
@tanmoyAtb tanmoyAtb self-assigned this Mar 31, 2022
@asnaith
Copy link
Member

asnaith commented Mar 31, 2022

However the requested changes in 2, 3 didn't feel like it was improving UX, I left them out.
Let me what you think if 2 or 3 are needed.

I agree, doesn't seem necessary.

Changes look great. Nice improvement to the mobile menu's especially. I thought they would use the same colour but this looks better.

Screen Shot 2022-03-31 at 2 12 12 PM

@Tbaut
Copy link
Collaborator

Tbaut commented Apr 1, 2022

This is a bit conflicting with the design discussed here #1284 but I think this looks cool too

@tanmoyAtb
Copy link
Contributor Author

This is a bit conflicting with the design discussed here #1284 but I think this looks cool too

Yup, I thought this version is more comfortable to the eyes.

Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

nice!

@Tbaut Tbaut enabled auto-merge (squash) April 1, 2022 10:49
@Tbaut Tbaut merged commit 17b595e into dev Apr 1, 2022
@Tbaut Tbaut deleted the mnt/focus-hover-styles-1678 branch April 1, 2022 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[a11y nits] Use sufficient contrast color for hover/focus
3 participants