fix(ui-modal): make Modal's header non-sticky with small window height #1854
Conversation
|
| @@ -83,6 +83,7 @@ const generateStyle = ( | |||
| borderBottomWidth: '0.0625rem', | |||
| borderBottomStyle: 'solid', | |||
| borderBottomColor: componentTheme.borderColor, | |||
| ...(smallPortView ? { position: 'relative' } : {}), | |||
There was a problem hiding this comment.
I needed to add this because otherwise the close button starts scrolling with the text instead of staying in the Modal.Header
| return safeCloneElement(child, { | ||
| variant: variant, | ||
| overflow: (child?.props as { overflow: string })?.overflow || overflow | ||
| }) | ||
| } else { | ||
| return child |
There was a problem hiding this comment.
I moved this to the cloneChildWithProps function
| // Adds the <div> to the beginning of the filteredChildren array | ||
| if (headerAndBody.length > 0) { |
There was a problem hiding this comment.
I needed to put the header and the body into the same element in order to be able scroll them together, so it results in some minor DOM change.
| @@ -86,7 +86,8 @@ class Modal extends Component<ModalProps, ModalState> { | |||
|
|
|||
| this.state = { | |||
| transitioning: false, | |||
| open: props.open ?? false | |||
| open: props.open ?? false, | |||
| windowHeight: window.innerHeight | |||
There was a problem hiding this comment.
We should always put null checks to DOM APIs because they dont exist when the user uses SSR. You can use our canUseDOM function to check whether its rendered on the server. Its safe to use them in componentDidMount AFAIK (please double check!), but definitely not in a static method like here
There was a problem hiding this comment.
I set windowHeight to 99999 in the state.(TypeScript didn't allow me not to define it). Also replaced a check with the canUseDOM.
Closes: INSTUI-4391
ToMESSKa
force-pushed
the
INSTUI-4391_make_modal_nicer_on_400_zoom
branch
from
897784f to
232e1cb
Compare
Closes: INSTUI-4391
ISSUE: Modal's header and footer are sitcky, so Modal.Body's content is hard to see and scroll on high zoom and narrow window, so it would be better if head become non-sticky according to the accessibility (A11y) team
TEST PLAN:
accessibility (A11y) team should review the component
also the exact breakpoint is still up for debate, I choose 20 rem now