chore(connection-form): clean up connection form styles; address modal jumping behavior #5836
Conversation
… stretch connection modal to the full available height when advanced is expanded
|
On a wide screen, the form gets very close to the bottom - maybe we can add min height? 
|
|
I don't think wide screen is what affects it, seems like I missed re-adding a padding back after refactoring the layout, will fix, thanks for spotting |
| setOpenRef.current?.(newValue); | ||
| return newValue; | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
if I understand correctly, the localOpen is just to avoid having 'open' as a callback dependency? is that worth it since the component anyway re-renders when open changes? am I missing something?
There was a problem hiding this comment.
Not really, localOpen is needed so that the accordion can be used without the need to control the state, this is the common controlled / uncontrolled pattern for React components. In leafygreen they usually use open, setOpen as the optional props to control the state, if you're writing a input in React for example, you similarly don't need to pass it value and html elements would still work as expected
There was a problem hiding this comment.
ah I see, thanks for the explanation!
| width: '100%', | ||
| paddingLeft: spacing[1], | ||
| paddingRight: spacing[1], | ||
| const formHelpConatinerStyles = css({ |
There was a problem hiding this comment.
nit: s/formHelpConatinerStyles/formHelpContainerStyles
There was a problem hiding this comment.
Thanks for spotting, fixed!
| paddingLeft: spacing[1], | ||
| paddingRight: spacing[1], | ||
| const formHelpConatinerStyles = css({ | ||
| position: 'sticky', |
There was a problem hiding this comment.
Don't know why I didn't think of or realise you can do position sticky inside here. I mean obviously when I think about it, but for some reason in my mind it was a relative to the screen thing.
|
@paula-stacho @lerouxb fixed the missing spacing at the bottom in two commits: first one just fixes a few inconsistencies in the existing form spacing so that it's easier to consistently apply bottom spacing and the next commit adds it |
because margin doesn't add scrollable space
Removes all the workarounds for the connection form sizing and changes them to css instead. Also a new behavior is added that makes the form to stretch to the whole available screen size when in modal to avoid jumps when switching the tabs in the advanced section
Kapture.2024-05-23.at.15.58.48.mp4