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

Thomas Dillard - Frontend Challenge #83

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

indifferentghost
Copy link

This pull request adds a responsive and accessible modal.

The component meets the following requirements:

  • Focus Management: The modal traps focus within itself, preventing users from tabbing out of it.
  • Background scroll-locking: The modal locks the background scrolling, preventing users from scrolling while the modal is open.
  • Tab navigation: The modal allows for tab navigation within itself, allowing users to easily navigate through the content.
  • React portals: The modal uses React portals to ensure that the modal is rendered outside of the main application flow.
  • Multi-modal environment: (see below)
  • Accessibility*: Could be improved (see below)
  • Mobile: The modal is optimized for mobile devices, with a responsive design that ensures it works well on small screens.

Although the modal currently lacks multi-modal environment support, implementing this feature could start by abstracting the state of the modals to a common parent, similar to how the React docs example manages multiple refs. This could include adding a context and state handler, or using popular libraries like jotai or other atomic state management systems.

While the modal component includes basic aria and role attributes on most required elements, further improvements may include:

  • Returning to the last focused element (before opening the modal) after closing the modal
    • event.relatedTarget may hold the secrets
  • A Modal.Title component that satisfies aria-labeledBy

Further improvements might include:

  • Gating modal management behind an API requiring less overhead (solved in implementing a multi-modal environment)
  • Including a default theme (mostly to standardize size and units)

Here's a loom video that provides a TL;DR: https://www.loom.com/share/1f441bf6a35e403e940cab1645be2aba

I'm excited to hear more!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant