Dialog

[T-Hugs]

This PR adds a new Dialog component to Primer React. This component is intended to replace the existing Dialog component (found at src/Dialog.tsx). For more details, see #1065 or read the rendered documentation.

Closes#1065

Open questions: (CC @vdepizzol)

1. How does “auto” width and height work? Is a dialog’s size (width or height) allowed to change after it is rendered?
2. Should we have a closing animation?
3. I added a 200ms fade in animation for the backdrop. Let me know if I should remove it!

Screenshots

Merge checklist

• Added/updated tests
• Added/updated documentation
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/primer/primer-components/F5HTpyU3VzY2ZZfG5pusKheZakAy
✅ Preview: https://primer-components-git-dialog-primer.vercel.app

[changeset-bot]

⚠️ No Changeset found

Latest commit: 084fbf9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[dgreif]

Awesome work! It's really nice that you got this working with pure styles and didn't even need useAnchoredPosition. 🚀

[colebemis]

Some of the text in the documentation example is not visible in dark mode:

I think setting the text color on the Dialog component will fix that issue:

Suggested change

	background-color: ${get('colors.bg.overlay')};
	color: ${get('colors.text.primary')};
	background-color: ${get('colors.bg.overlay')};

[T-Hugs]

I can make this change, but the real fix IMO is to have <BaseStyles> use theme values.

[T-Hugs]

On a second look, it seems like this is an issue only in the docs.

Does this have something to do with these comments found in wrap-page-element.js?

// Temporarily shadowing this file to wrap the page in our custom ThemeProvider.
// We can remove this file when Doctocat uses the custom ThemeProvider.
// Doctocat still uses presentional color variables.
// We can remove this customTheme when Doctocat uses functional color variables

[colebemis]

This is looking really great, @T-Hugs! This API seems like a nice balance between providing a happy path and escape hatches.

[T-Hugs]

Awesome work! It's really nice that you got this working with pure styles and didn't even need useAnchoredPosition. 🚀

Haha, yeah, it was never my intention to use getAnchoredPosition to implement dialog (sorry if I deceived 😛). Just wanted to start with that in order to gain a bit more mileage with it before swapping to a pure CSS solution.

[colebemis]

🚀