airbyte
airbyte copied to clipboard
🪟🐛 Make modal scrollable
What
Fixes #21793
Currently, the modal component will overflow the viewport and parts of the content will be cut off. This PR allows for the modal to be limited to the currently available height and the body of the modal becoming scrollable:
https://user-images.githubusercontent.com/1508364/215099643-85830214-86ed-47d9-95bc-b379623de7f4.mp4
How
- Make all levels of the modal component hierarchy flex
- Set a max-width on the wrapper centering the modal
- Inputs form had to be adjusted because it wrapped the form element around the body and the footer so the body didn't have a chance to shrink anymore on too much content - moved it outside of the wrapper.
I checked a few other modals around the app and didn't notice any breakages, but worth checking them again in case I missed something
@flash1293 this PR doesn't seem to solve the issue for me. It just locks the top of the modal to the top of my screen, but the bottom of the window still covers up the modal instead of the modal shrinking.
Here is a screen recording of what I'm seeing on this branch:
https://user-images.githubusercontent.com/22731524/215893944-fd3b5551-0718-48ed-be7d-8f4e4bf31b2e.mov
I tried fiddling with the CSS a bit more but wasn't able to affect this behavior quickly. Are you able to reproduce what I'm seeing?
I think I messed something up before submitting, sorry - could you check again? Two other things I noticed:
- Had to move the form back into the modal because otherwise it's not wrapping the submit buttons in the dom and submitting doesn't work
- This fix doesn't work for the test input form because it nests the footer in the body due to the dom structure - I think it's fixable but it's a bit more refactoring, I will do that separately
Good callout @lmossman - my thinking was that the scrolling works as long as header, body and footer and in the same parent element, but that's often not the case because of the way we do forms - I worked around this by introducing an optional wrapIn
prop to the modal component which allows the caller to wrap header/body/footer into the the required form component on the same level, so scrolling should work for these two cases now as well (without css tricks)