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

Notifications, showing, short selector a11y #2711

Closed
wants to merge 13 commits into from

Conversation

yeion7
Copy link
Contributor

@yeion7 yeion7 commented Oct 8, 2019

What kind of change does this PR introduce?

Refactor, reopen #2652

What is the current behavior?

The notifications has a few a11 issues

What is the new behavior?

What steps did you take to test this?

Checklist

  • Documentation
  • [] Testing
  • Ready to be merged
  • Added myself to contributors table

@vercel
Copy link

vercel bot commented Oct 8, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://codesandbox-client-git-fork-yeion7-notifications-a11y.codesandbox1.now.sh

@vercel vercel bot temporarily deployed to staging October 8, 2019 15:36 Inactive
@yeion7 yeion7 changed the title Notifications, showing, short selector a11y [WIP] Notifications, showing, short selector a11y Oct 8, 2019
@yeion7
Copy link
Contributor Author

yeion7 commented Oct 8, 2019

@CompuIves I've reopened this from #2652, maybe we could find a solution for outline styles and merge this change, sorry for the prev PR

Could you check if the z-index and performance issues persist after upgrade reakit? pls

@yeion7
Copy link
Contributor Author

yeion7 commented Oct 8, 2019

This is the current outline, maybe we could define a better outline style or maybe change the item background on hover

image

image

@yeion7 yeion7 force-pushed the notifications-a11y branch from d2de254 to ac8f7d1 Compare October 8, 2019 15:51
@vercel vercel bot temporarily deployed to staging October 8, 2019 15:51 Inactive
@vercel vercel bot temporarily deployed to staging October 8, 2019 16:32 Inactive
@yeion7 yeion7 changed the title [WIP] Notifications, showing, short selector a11y Notifications, showing, short selector a11y Oct 8, 2019
@vercel vercel bot temporarily deployed to staging October 9, 2019 12:13 Inactive
@yeion7
Copy link
Contributor Author

yeion7 commented Oct 9, 2019

@CompuIves what do you think about this? use the same styles as user menu on hover items

@diegohaz
Copy link

diegohaz commented Oct 17, 2019

Read on the other PR that there was some performance issue! Is this still an issue? If so, please let me know if there's anything I can help with on the Reakit side. :)

@yeion7
Copy link
Contributor Author

yeion7 commented Oct 24, 2019

@CompuIves any chance of moving this PR forward?

@lbogdan
Copy link
Contributor

lbogdan commented Dec 11, 2019

Build for latest commit 435bc57 is at https://pr2711.build.csb.dev/s/new.

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.

4 participants