Skip to content

fix(full-height): reset the frame height when a different Liveboard opens - #674

Open
sastaachar wants to merge 1 commit into
mainfrom
SCAL-332771-liveboard-height-reset
Open

sastaachar wants to merge 1 commit into
mainfrom
SCAL-332771-liveboard-height-reset

Conversation

@sastaachar

Copy link
Copy Markdown
Contributor

What

In a full-height embed, reset the iframe height to its default when the app navigates from one Liveboard to another.

FullHeightController.handleRouteChange only reset the height when the app left the Liveboard experience altogether. Any route in LIVEBOARD_RELATED_ROUTES returned early, so Liveboard → Liveboard kept the height of the board being left behind:

if (LIVEBOARD_RELATED_ROUTES.some((route) => currentPath.startsWith(route))) {
    return;   // LB1 -> LB2 never resets
}

Why it matters

A full-height frame is expanded to the whole Liveboard, so after a tall board the frame is left thousands of pixels tall. Carry that into the next Liveboard and the embedded app's own window.innerHeight is still the previous board's height. Anything that falls back to the iframe viewport as its IntersectionObserver root then treats the entire board as on-screen and resolves every visualization at once — so lazyLoadingForFullHeight quietly stops taking effect from the second Liveboard onward, which is what a customer reported as a 22s load on a 32-tile board.

How

  • Track the Liveboard the frame is currently sized for, read as the first GUID in the route (every route in the list carries the Liveboard's GUID first, so /embed/viz/<liveboard>/<viz> resolves to the Liveboard).
  • Reset to frameParams.height, or the minimum height, when a different Liveboard opens.
  • A route change within one Liveboard is left alone — the height is already right for it, and resetting would make the frame jump while drilling into a visualization.
  • The first Liveboard of a session does not reset: there is nothing to reset from, and the frame is still at its start height.

Tests

src/full-height.spec.ts — 7 new cases under RouteChange › moving between Liveboards. Three of them fail if the source change is reverted (verified):

  • resets the height when a different Liveboard is opened
  • resets to frameParams.height when one is configured
  • reads the Liveboard from the first GUID of a visualization route

The rest pin the cases that must NOT reset: the first Liveboard, navigation within one Liveboard, and the board opened after leaving the experience.

Existing RouteChange tests are unaffected — they use non-GUID paths such as /pinboard/abc, which carry no Liveboard id and so keep their current behaviour.

Full suite: 1859 passed, 46 suites, 4 skipped. tsc and eslint clean.

Notes

This is one of two defects found under SCAL-332771. The other — that postRender() is reachable only from render(), so a pre-rendered embed attaches no scroll/resize listeners at all — is a separate change and is not in this PR.

Ref: SCAL-332771

🤖 Generated with Claude Code

…pens

A full-height frame is expanded to the whole Liveboard, so after a tall board
the frame is left thousands of pixels tall. RouteChange only reset the height
when the app left the Liveboard experience altogether; moving from one Liveboard
to another returned early and kept the previous board's height.

That carries the old height into the new board: the embedded app's own
window.innerHeight is the tall board's height, so anything falling back to the
iframe viewport treats the whole board as on-screen and resolves every
visualization at once — the lazy loading the host app asked for quietly stops
happening on the second Liveboard.

Track the Liveboard the frame is sized for and reset to frameParams.height, or
the minimum height, when a different one opens. A route change within the same
Liveboard is left alone so the frame does not jump while drilling into a
visualization.

Ref: SCAL-332771

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@sastaachar
sastaachar requested a review from a team as a code owner September 16, 2026 12:36

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

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.

Code Review

This pull request updates the FullHeightController to reset the frame height when navigating between different Liveboards, which prevents lazy loading issues caused by carrying over a tall Liveboard's height to a new one. It introduces a regex to extract the Liveboard ID from the route path and resets the height if the ID changes. Comprehensive unit tests have been added to verify this behavior. The review feedback recommends normalizing the extracted Liveboard GUID to lowercase to ensure case-insensitive comparison robustness.

Comment thread src/full-height.ts
Comment on lines +306 to +312
const liveboardId = LIVEBOARD_ID_IN_PATH.exec(currentPath)?.[0];
if (!liveboardId) {
return;
}
const isDifferentLiveboard = this.currentLiveboardId !== undefined
&& this.currentLiveboardId !== liveboardId;
this.currentLiveboardId = liveboardId;

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

Comparing GUIDs directly without case normalization can lead to unexpected behavior if the URL contains uppercase hex characters (e.g., A3E48B45-... vs a3e48b45-...). Normalizing the extracted liveboardId to lowercase ensures robust case-insensitive comparisons.

Suggested change
const liveboardId = LIVEBOARD_ID_IN_PATH.exec(currentPath)?.[0];
if (!liveboardId) {
return;
}
const isDifferentLiveboard = this.currentLiveboardId !== undefined
&& this.currentLiveboardId !== liveboardId;
this.currentLiveboardId = liveboardId;
const liveboardId = LIVEBOARD_ID_IN_PATH.exec(currentPath)?.[0]?.toLowerCase();
if (!liveboardId) {
return;
}
const isDifferentLiveboard = this.currentLiveboardId !== undefined
&& this.currentLiveboardId !== liveboardId;
this.currentLiveboardId = liveboardId;

@pkg-pr-new

pkg-pr-new Bot commented Sep 16, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 2c19f77

This branch has not been deployed

No deployments
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.

1 participant