Fix: Add max height to dialog's content to prevent cut off

[fluiddot]

Fix

Fixes: #2750

If a dialog overflows the height of the window, the content of the dialog is cut off.

max-height-dialog-content.mp4

Test

1. Click the Menu button.
2. Click the "Keyboard Shortcuts" button.
3. Zoom in the window a couple of times (can be done via View/Zoom In).
4. Decrease the height of the browser window until it's smaller than the dialog height.
5. Observe that the dialog's content is not cut off and can be scrolled.

Release

Fixed cut off issue for dialogs when the window is too small.

[fluiddot]

The max height of the content view is calculated by subtracting the header height (56px) and some padding (20px) to the window height (100vh).

[sandymcfadden]

Thanks so much for diving in and starting on fixes!
There are a couple of small things. In a previous PR, I was working on one dialog in particular and took a similar approach to what you did here to set the max height. It was decided instead that we could use a height of 420px as the max instead.
The dialog in your video works well as one of the inner elements of the dialog has an overflow set for it. For other dialogs this can cause the content to overflow out of it. Would be good to add an overflow: auto; to this as well.
Not sure how far you want to go with this but #2777 is sort of related to this as well.
The plan was to have the entire height of the dialog be a max of 420px, then have the inner content be scrollable if needed. The header, and if there were any action buttons with the dialog at the bottom, would remain static and always visible without scrolling.

[fluiddot]

There are a couple of small things. In a previous PR, I was working on one dialog in particular and took a similar approach to what you did here to set the max height. It was decided instead that we could use a height of 420px as the max instead.

I just checked that the minimum window is set to 520px so 420px should be enough. However the content is still cut off if you zoom in the UI (see attached video), as far as I checked the only way to prevent it is by using the viewport height with vh unit 😞 .

dialog-content-zoom-in.mp4

The dialog in your video works well as one of the inner elements of the dialog has an overflow set for it. For other dialogs this can cause the content to overflow out of it. Would be good to add an overflow: auto; to this as well.

Good point! I think it would be a great idea to add overflow to the content view, this way we assure that it's always scrollable.

Another thing it would be interesting is to review the dialogs and set overflow to the sections that would require it. As an example, I'm working on this issue related UnsynchronizedConfirmation dialog and here I think makes more sense to make the changes list scrollable not the whole content (check follow video).

unsynchronized-dialog-overflow.mp4

Not sure how far you want to go with this but #2777 is sort of related to this as well.

I'm still getting familiar with the project but I can take a look if you want.

[fluiddot]

One option I'm considering, although I'm not sure about potential compatibility issues, is to use this for calculating the max height:
max-height: unquote('min(100vh - 20px, 420px)');.

This will allow us to set the maximum height to 420px but if for any reason the window gets smaller (looks like this is what's happening when zooming in), we use the window height to prevent the cut off.

Note the use of unquote for prevent using the Sass's min function because it doesn't support vh units.

[sandymcfadden]

Using the min function seems like a good approach to me here to get the 420px height, but also ensure it works on zoomed in views.

My thoughts on #2777 were very similar to what you're thinking about for the UnsynchronizedConfirmation dialog.

[fluiddot]

After a second thought, I decided to use media queries instead of this approach because it looks way safer (related commit).

EDIT: I've also updated the video from the description.

[sandymcfadden]

Thanks so much for diving in and starting on fixes!
There are a couple of small things. In a previous PR, I was working on one dialog in particular and took a similar approach to what you did here to set the max height. It was decided instead that we could use a height of 420px as the max instead.
The dialog in your video works well as one of the inner elements of the dialog has an overflow set for it. For other dialogs this can cause the content to overflow out of it. Would be good to add an overflow: auto; to this as well.
Not sure how far you want to go with this but #2777 is sort of related to this as well.
The plan was to have the entire height of the dialog be a max of 420px, then have the inner content be scrollable if needed. The header, and if there were any action buttons with the dialog at the bottom, would remain static and always visible without scrolling.

[sandymcfadden]

I left one comment about the use of interpolation, but other than that this looks good, thank you! I like the media query approach.
This tests well on all the dialogs I tried where it doesn't fall out of the screen and I can access all the content in it.