picocss / pico

Minimal CSS Framework for semantic HTML
https://picocss.com
MIT License
13.71k stars 399 forks source link

dialog article max-width #127

Closed prologic closed 2 years ago

prologic commented 2 years ago

What's the purpose of these media selectors and max-width on the dialog article element?

https://github.com/picocss/pico/blob/6b05b161ef3a00e2e9b1135b88e302a4c32f4519/css/pico.classless.css#L1897-L1906

In our use case, this causes the dialog to overflow the bounds of the view port.

With max-width enabed:

Screen Shot 2022-01-09 at 23 54 04

Without:

Screen Shot 2022-01-09 at 23 54 28
lucaslarroche commented 2 years ago

Hi @prologic,

It's a common practice to constraint the width of a modal to avoid gigantic modal on large screens. I don't see a bug here. What is your problem? Is it because your page viewport does not appear to be centered with the modal?

prologic commented 2 years ago

The problem here as you can see in the 1st screenshot, it the modal itself is outside of the bounds of the viewport.

This I believe to be a slight bug?

prologic commented 2 years ago

Specifically the top and bottom are getting cut off as is the contained image. It might be something we're doing wrong though in our use of the <dialog> so I not 100% sure it's a bug here to be fair.

lucaslarroche commented 2 years ago

Ok, I was looking at the width... but looking at the height: yes we might have a bug (unhandled case). I did some tests and if the content is taller than the screen we don't have any scroll.

Note for myself:

max-height: calc(100vh - var(--spacing) * 2);
overflow: auto;
cara-tm commented 2 years ago

Exact ! You were faster than me; I was looking a solution:

dialog article {
    overflow-y: auto;
    max-width: 510px;
    max-height: calc(100% + var(--block-spacing-vertical))
}