hypothesis / hypothesis/frontend-shared

New `Dialog` component wrapping a native `dialog`

Open
#1,461 0 comments 0 reactions 0 assignees View on GitHub
component
Dominant language
TypeScript
Stars
8
Forks
5
PR merge metrics
No merged PRs in 30d

Description

The support for the native `dialog` is reaching all the browsers we support. This is a good opportunity to build a new `Dialog` component that makes use of it, and allows us to drop some of the custom logic.

We could just refactor the existing `Dialog` and `ModalDialog` components, but creating a new one instead, would allow us to apply some architectural changes and not having to keep a 100% compatible API, while we slowly adopt it in downstream projects.

### Some of the changes I think this new component should introduce:

#### Handle its open/closed state via an `open` prop, instead of via mounting/unmounting.

This will better fit the side effects we need to trigger in order to show/hide the native dialog.

It will also help solving an existing problem with current components when transition components are passed. We tried to solve that problem this way some time ago, and it proved challenging, but with a simplified component it will probably be easier to handle.

Ultimately, most of the component libraries we usually get inspiration from, use some kind of prop handled by consumer code to determine if the component should show or not.

#### Do not implicitly wrap a `Panel`.

Existing `Dialog` started as a wrapper around `Panel`, which became a problem later when we tried to use it for other components providing its own "layout".

Instead, we should have a simpler Dialog, and if needed, a higher level `PanelDialog` wrapping the new Dialog and implicitly rendering a `Panel` or `Card`.

### Potential API changes that we might want to introduce, but need some testing and experimentation first:

#### Single `Dialog` component for modal and non-modal contexts.

Due to the amount of custom logic in existing implementations, we have two separate components, `Dialog` and `ModalDialog`.

Thanks to the amount of logic we will be able to get rid of when using the native `dialog`, maybe we can have a single component with a `modal={true|false}` prop.

### Use cases

An inspiration for this component can be found in the client, where we recently used the native `dialog` with a very small fraction of the API we might want to support, to render the notebook and the user profile: https://github.com/hypothesis/client/pull/6207

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.