don't give normal and large modals a fixed height #2610
Conversation
Signed-off-by: szaimen <szaimen@e.mail.de>
szaimen
requested review from
nickvergessen,
jancborchardt,
artonge,
skjnldsv,
Pytal and
marcoambrosini
| @@ -735,15 +735,13 @@ export default { | |||
| max-width: 90%; | |||
| width: 600px; | |||
| max-height: 90%; | |||
| height: 600px; | |||
There was a problem hiding this comment.
maybe we should give it a min-heigt of e.g. 300px now that we don't enforce a specific size anymore?
There was a problem hiding this comment.
What about a width: clamp(300px, 600px, 90vw);?
There was a problem hiding this comment.
width is not for discussion... do you mean height?
There was a problem hiding this comment.
Yeah right, off topic. I just stumbled upon this definition, because I prefer clamb, min and max over min-width, max-width, ... .
Had to be commented under the next comment. 😁
There was a problem hiding this comment.
okay, feel free to open a PR for discussion. For me using min, max, etc. is fine :)
There was a problem hiding this comment.
Ah, no. That's not valuable, because the result is the same. Just consumes time. For me this are just small 'by the way' improvements of the css code. I am more satified that I can remove my height hack after the release.
|
and while we are at it, what do you think about changing the width and max-width here: |
|
@marcoambrosini removing the fixed height should just fall-back to the height of the elements inside (aka |
|
🤔 couldn't it be that someone is relying on that height for scrollable content? |
I don't think so... |
I would say the opposite is the case. I was relying on the fit-content-height and had to fix the forced height in every modal to get the behavior before this change was introduced. |
|
So merge then or what are we waiting for? (the initial problem should be fixed with this change) |
Fix nextcloud/server#31854
Signed-off-by: szaimen szaimen@e.mail.de