Skip to content

Nested scrollable container with lazyloading and fullheight#542

Open
ruchI9897 wants to merge 6 commits into
mainfrom
lazyload-fullheight
Open

Nested scrollable container with lazyloading and fullheight#542
ruchI9897 wants to merge 6 commits into
mainfrom
lazyload-fullheight

Conversation

@ruchI9897
Copy link
Copy Markdown
Contributor

@ruchI9897 ruchI9897 commented May 27, 2026

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 iframe
The 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: true
When 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.

@ruchI9897 ruchI9897 requested a review from a team as a code owner May 27, 2026 05:16
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/embed/app.ts
Comment thread src/embed/liveboard.ts
Comment thread src/utils.ts
Comment thread src/utils.ts
Comment on lines +542 to +544
export const getScrollableAncestors = (element: HTMLElement) => {
const ancestors: HTMLElement[] = [];
let parent = getParentElementAcrossShadowRoot(element);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add a defensive check to ensure element is defined before attempting to find its scrollable ancestors.

export const getScrollableAncestors = (element: HTMLElement) => {
    const ancestors: HTMLElement[] = [];
    if (!element) {
        return ancestors;
    }
    let parent = getParentElementAcrossShadowRoot(element);

Comment thread src/utils.ts
Comment thread src/utils.ts Outdated
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 27, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@thoughtspot/visual-embed-sdk@542

commit: 2cfd418

@ruchI9897 ruchI9897 changed the title fix Nested scrollable container with lazyloading and fullheight May 27, 2026
ruchI9897 and others added 2 commits May 27, 2026 11:22
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@shivam-kumar-ts
Copy link
Copy Markdown
Contributor

@gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/embed/liveboard.ts
Comment thread src/utils.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants