Should Dialog let contents overflow outside?

[siddharthkp]

When an element inside <details-dialog> is longer than the dialog, it results in a scroll on the dialog which isn't ideal:

https://user-images.githubusercontent.com/1863771/158603253-bedb71b2-9115-4ab2-be9d-21f9bd7876ed.mov

This can be reproduced in the Dialog with <include-fragment> example: https://github.github.io/details-dialog-element/example/index.html

https://user-images.githubusercontent.com/1863771/158602534-d440a0d9-91b1-454a-9695-80a0599e03f3.mov

The cause can be narrowed down to a overflow:auto index.css#L12

Removing that line, does make it behave nicely. But, I'm not sure what would break instead OR if there are any tradeoffs.

https://user-images.githubusercontent.com/1863771/158603267-e7906134-75fe-48fc-9e1b-9a4d240cc19d.mov

More screenshots via @dipree:

[dipree]

The overflow: auto was introduced a while back and the reason is documented here https://github.com/github/details-dialog-element/pull/63. But the version with this change (3.1.3) was only recently (9 days ago) introduced by https://github.com/github/github/commit/77a0d5b478d346e830cec5f9284effa156f834dd.

overflow: auto will solve the problem of allowing to access the content when the viewport is too small but it will also cause elements that should overflow, like a dropdown or select menu, to be swallowed by the dialog.

[dipree]

What triggered the version bump https://github.com/github/codespaces/issues/6834

[pqt]

Something I want to note here is that in your third video demonstrating the overflow, you scroll but not so much as to scroll the page, in #63 that is the problem being targeted (and solved) by the overflow: auto in the PR.

I'd personally be advocating for the contents to not overflow by default, however, in the case where an unknown number of options can appear in a dropdown it would make sense to reset the overflow to visible (.overflow-visible) on a case-by-case basis.

[siddharthkp]

@pqt Good shout!

Looking at #63, feels like we might have a regression?:

(changed the height of the dialog with developer tools)

https://user-images.githubusercontent.com/1863771/159266938-635d170a-c27b-4621-9c4a-264ddb302f22.mov

---

Scrolling on the transfer issue dialog:

1. with overflow:auto (this is on production today)

   You can scroll all the sections on the page:

   https://user-images.githubusercontent.com/1863771/159264787-a266440b-d6c5-4571-80d2-840c6785ffbe.mov

2. without overflow: auto

   https://user-images.githubusercontent.com/1863771/159265167-f19df277-ff0b-4187-8e86-21158b229000.mov