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

fix: Keyboard accessible context menus #1768

Merged
merged 34 commits into from Jan 14, 2021
Merged

fix: Keyboard accessible context menus #1768

merged 34 commits into from Jan 14, 2021

Conversation

tommoor
Copy link
Member

@tommoor tommoor commented Jan 5, 2021

  • Makes menus fully accessible and keyboard driven
  • Currently adds 2.8% to initial bundle size due to the inclusion of Reakit and its dependency, popperjs.
  • Converts all menus to functional components
  • Remove old custom menu system
  • Various layout and flow improvements around the menus

closes #1766

@tommoor tommoor changed the title fix: Accessible menus (Proof of concept) fix: Keyboard accessible context menus Jan 11, 2021
@tommoor tommoor marked this pull request as ready for review January 11, 2021 05:46
@tommoor
Copy link
Member Author

tommoor commented Jan 11, 2021

This still isn't mergable as it's not as performant due to the menus rendering all of their contents before they are clicked, which is different to the old system. I think in order to get this feeling good the key menus that are on every page (DocumentListItem, DocumentLink, CollectionLink) will need to be rendered only when visible which will neccessitate moving hover/focus into React state.

@@ -42,7 +46,7 @@ const RealButton = styled.button`
}

${(props) =>
props.neutral &&
props.$neutral &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does the leading $ mean generally?

Copy link
Member Author

Choose a reason for hiding this comment

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

Stops it being added to the dom by styled-components: https://styled-components.com/docs/api#transient-props

@tommoor tommoor merged commit e8b7782 into develop Jan 14, 2021
@delete-merged-branch delete-merged-branch bot deleted the fix/issue-1766 branch January 14, 2021 06:00
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.

Refactor popover menus to use Reakit
2 participants