[EuiCallOut] Use resize observer instead of container queries#9727
[EuiCallOut] Use resize observer instead of container queries#9727mgadewoll wants to merge 3 commits into
Conversation
- showcases behavior in different container contexts
💚 Build Succeeded
cc @mgadewoll |
💚 Build Succeededcc @mgadewoll |
|
I'll be reviewing this later today! |
acstll
left a comment
There was a problem hiding this comment.
🟢 Container responsiveness works as expected. I know we discussed this offline but I don't think it hurts to express my disappointment with container queries again 🫠 (let's try and make some time to investigate in this regard in the near future).
I left a suggestion to simplify the useLayoutObserver hook for your consideration (I tested it locally). Also, I think it'd be worth considering adding a small unit test for the hook.
| /* Uses resize observer to determine the container width/layout instead of native container queries, | ||
| because callouts can be placed in containers without defined size (absolute positioned, no-grow flex layout etc.) | ||
| where container queries would collapse by design instead of adjusting to the content dimensions. */ | ||
| const panelRef = useLayoutObserver(size, ref); |
There was a problem hiding this comment.
[non-blocking] I have a suggestion for simplifying refs by combining them here and removing the logic from the hook (follow-up comment)
the import for useCombinedRefs from services is missing in this diff, the suggestion below is for illustrating, not directly committing 😛
| const panelRef = useLayoutObserver(size, ref); | |
| const layoutRef = useLayoutObserver(size); | |
| const panelRef = useCombinedRefs([layoutRef, ref]); |
| export const useLayoutObserver = ( | ||
| size: EuiCallOutSize, | ||
| ref: React.ForwardedRef<HTMLDivElement> | ||
| ): ((node: HTMLDivElement | null) => void) => { | ||
| const elementRef = useRef<HTMLDivElement | null>(null); | ||
| const forwardedRefRef = useRef(ref); | ||
| forwardedRefRef.current = ref; | ||
|
|
||
| const panelRef = useCallback((node: HTMLDivElement | null) => { | ||
| elementRef.current = node; | ||
| const fRef = forwardedRefRef.current; | ||
| if (typeof fRef === 'function') { | ||
| fRef(node); | ||
| } else if (fRef) { | ||
| (fRef as React.MutableRefObject<HTMLDivElement | null>).current = node; | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
following up on the previous comment, by passing no ref you can remove the useCallback bit:
| export const useLayoutObserver = ( | |
| size: EuiCallOutSize, | |
| ref: React.ForwardedRef<HTMLDivElement> | |
| ): ((node: HTMLDivElement | null) => void) => { | |
| const elementRef = useRef<HTMLDivElement | null>(null); | |
| const forwardedRefRef = useRef(ref); | |
| forwardedRefRef.current = ref; | |
| const panelRef = useCallback((node: HTMLDivElement | null) => { | |
| elementRef.current = node; | |
| const fRef = forwardedRefRef.current; | |
| if (typeof fRef === 'function') { | |
| fRef(node); | |
| } else if (fRef) { | |
| (fRef as React.MutableRefObject<HTMLDivElement | null>).current = node; | |
| } | |
| }, []); | |
| export const useLayoutObserver = ( | |
| size: EuiCallOutSize | |
| ): RefObject<HTMLDivElement | null> => { | |
| const elementRef = useRef<HTMLDivElement | null>(null); |
| return () => observer.disconnect(); | ||
| }, [size]); | ||
|
|
||
| return panelRef; |
There was a problem hiding this comment.
if you apply the above:
| return panelRef; | |
| return elementRef; |
Summary
This PR is a follow-up to #9705, which updated
EuiCallOutto use container queries to handle the responsive behavior of the action elements.Because
EuiCallOutis a somewhat general component that can be placed in different layout contexts, it might be placed in containers that have no intrinsic width which would lead the container query to collapse as it has no dimensions to align to (e.g. absolute positioned containers - like<EuiPopover>- or flex layouts with no width basis - like<EuiFlexItem grow={false}>To prevent issues because of this, we instead use a minimal resize observer implementation that sets a
data-layoutattribute which CSS can target accordingly instead of the container query.API Changes
⚪ No API changes
Screenshots
For Callouts in general there no visual changes expected.
Impact Assessment
Note: Most PRs should be tested in Kibana to help gauge their Impact before merging.
Impact level: 🟢 None - internal change only (pre-rollout)
Release Readiness
QA instructions for reviewer
Checklist before marking Ready for Review
QA: Tested docs changesChangelog: Added changelog entryBreaking changes: Addedbreaking changelabel (if applicable)Reviewer checklist