Nested scrollable container with lazyloading and fullheight#542
Nested scrollable container with lazyloading and fullheight#542ruchI9897 wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request enhances full-height lazy loading for both App and Liveboard embeds by listening to scroll and resize events on scrollable and clipping ancestor elements, rather than just the window. It introduces utility functions to traverse the DOM (including shadow roots) to identify these ancestors and updates visibility calculations accordingly. The code review feedback recommends adding defensive checks across several utility functions and embed classes to ensure that elements, particularly this.iFrame, are defined before accessing their properties, thereby preventing potential runtime TypeErrors.
| export const getScrollableAncestors = (element: HTMLElement) => { | ||
| const ancestors: HTMLElement[] = []; | ||
| let parent = getParentElementAcrossShadowRoot(element); |
There was a problem hiding this comment.
commit: |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
@gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces container-aware full height lazy loading for the App and Liveboard embeds, allowing the SDK to listen to scroll and resize events from scrollable or clipping ancestor containers instead of only the browser window. The review feedback highlights two critical issues: a potential crash in LiveboardEmbed when registering lazy load events if the iframe is not yet created, and a potential runtime TypeError in hasMatchingOverflow if window.getComputedStyle returns null.
Problem : If the embed is rendered inside a fixed-height scroll container within a shadow DOM. In this case, the actual viewport for the iframe is not the browser window; it is the customer-created scroll container:
#your-own-div -> shadowRoot -> scrollContainer height: 500px; overflow: auto -> embedTarget -> ThoughtSpot iframeThe current SDK lazy-loading logic listens to window scroll/resize events and calculates visible iframe coordinates against the browser viewport. However, when the user scrolls the inner scrollContainer, the browser window does not scroll. As a result, the SDK does not recompute/send updated VisibleEmbedCoordinates for the iframe, and lazy loading does not correctly load visualizations as they enter the container viewport.
Root Cause : The SDK assumes window is the only scrolling/visibility viewport for full-height lazy loading. This assumption breaks when the embed is placed inside a nested scrollable container, especially inside shadow DOM.
Solution : Added a SDK flag:
enableContainerAwareLazyLoadingForFullHeight: trueWhen this flag is enabled, the SDK finds the containers around the iframe that can scroll or hide part of it, even if the iframe is inside a shadow DOM. It then calculates what part of the iframe is actually visible inside that container, listens when the container is scrolled, and updates the visibility again if the container size changes.