airbyte icon indicating copy to clipboard operation
airbyte copied to clipboard

🪟🐛 Make modal scrollable

Open flash1293 opened this issue 1 year ago • 3 comments

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 avatar Jan 27 '23 13:01 flash1293

@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?

lmossman avatar Jan 31 '23 22:01 lmossman

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

flash1293 avatar Feb 01 '23 10:02 flash1293

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)

flash1293 avatar Feb 02 '23 12:02 flash1293