Dialog component

[mxstbr]

For our internal project, we will require a modal. I have to build it anyway, so I figured I'd kick off a discussion about upstreaming it to @primer/components!

Here is the API I was thinking about using:

<Dialog title="String" isOpen={true} onDismiss={() => {}}>
  <Text>Content</Text>
</Dialog>

This would render a dialog like this as a React portal:

Clicking either the X Octicon or the background overlay calls onDismiss, which should close the dialog.

Initial questions:

• Should the title prop be able to accept any React node, do we render e.g. icons in dialog titles anywhere?

• I specifically avoided adding padding automatically to the contents, as that would make it hard to e.g. render an alert in the dialog like we do in the "Delete repo" dialog (reference below). To make sure users set the spacing correctly, we could add a DialogBody component that does nothing but sets padding: 16px around its children?

  

• I would like to use @reach/dialog to make sure we get the a11y correct. Would that be alright?

[colebemis]

This API looks good to me! 👍 Thanks, @mxstbr 🎉

Should the title prop be able to accept any React node, do we render e.g. icons in dialog titles anywhere?

I could go either way on this one but I'm leaning towards constraining the title prop to only accept strings.

To make sure users set the spacing correctly, we could add a DialogBody component that does nothing but sets padding: 16px around its children?

I'm not opposed to a DialogBody component but <Box p={3}> is already pretty concise. If we're going for a "Minimal API," we can probably leave out DialogBody.

I would like to use @reach/dialog to make sure we get the a11y correct. Would that be alright?

@emplums would know better than me, but I'm fine with using @reach/dialog. I'm not sure if we have the proper loaders to import the Reach stylesheet (https://ui.reach.tech/styling). So you might have to set that up.

[mxstbr]

Agree with everything you said @colebemis! We should be able to integrate the Reach styles with styled-components fairly easily if we can import them as a string, I can definitely set that up.