Skip to content

fix #540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ruchI9897 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix #540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions src/embed/app.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1734,11 +1734,32 @@ describe('App embed tests', () => {
appEmbed.destroy();

expect(removeEventListenerSpy).toHaveBeenCalledWith('resize', expect.any(Function));
expect(removeEventListenerSpy).toHaveBeenCalledWith('scroll', expect.any(Function));
expect(removeEventListenerSpy).toHaveBeenCalledWith('scroll', expect.any(Function), true);

removeEventListenerSpy.mockRestore();
});

test('should skip lazy load data when iframe is unavailable', async () => {
const appEmbed = new AppEmbed(getRootEl(), {
...defaultViewConfig,
fullHeight: true,
lazyLoadingForFullHeight: true,
lazyLoadingMargin: '10px',
} as AppViewConfig);

const mockTrigger = jest.spyOn(appEmbed, 'trigger');

await appEmbed.render();

(appEmbed as any).iFrame = undefined;
(appEmbed as any).sendFullHeightLazyLoadData();

expect(mockTrigger).not.toHaveBeenCalledWith(
HostEvent.VisibleEmbedCoordinates,
expect.anything()
);
});

test('should handle RequestVisibleEmbedCoordinates event and respond with correct data', async () => {
// Mock the iframe element
mockIFrame.getBoundingClientRect = jest.fn().mockReturnValue({
Expand Down Expand Up @@ -1780,6 +1801,24 @@ describe('App embed tests', () => {
},
});
});

test('should not respond to RequestVisibleEmbedCoordinates when iframe is unavailable', async () => {
const appEmbed = new AppEmbed(getRootEl(), {
...defaultViewConfig,
fullHeight: true,
lazyLoadingForFullHeight: true,
lazyLoadingMargin: '10px',
} as AppViewConfig);

await appEmbed.render();

const mockResponder = jest.fn();

(appEmbed as any).iFrame = undefined;
(appEmbed as any).requestVisibleEmbedCoordinatesHandler({}, mockResponder);

expect(mockResponder).not.toHaveBeenCalled();
});
});

describe('IFrame height management', () => {
Expand Down Expand Up @@ -1966,4 +2005,3 @@ describe('AppEmbed visualOverrides tests', () => {
});
});


13 changes: 11 additions & 2 deletions src/embed/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@
* @example
* ```js
* // Replace <EmbedComponent> with embed component name. For example, AppEmbed or LiveboardEmbed
* const embed = new <EmbedComponent>('#tsEmbed', {

Check warning on line 601 in src/embed/app.ts

View workflow job for this annotation

GitHub Actions / build 

Comments may not exceed 90 characters
* ... // other embed view config
* isContinuousLiveboardPDFEnabled: true,
* })
Expand Down Expand Up @@ -1130,9 +1130,13 @@
}

private sendFullHeightLazyLoadData = () => {
if (!this.iFrame?.getBoundingClientRect) {
return;
}

const data = calculateVisibleElementData(this.iFrame);
// this should be fired only if the lazyLoadingForFullHeight and fullHeight are true
if(this.viewConfig.lazyLoadingForFullHeight && this.viewConfig.fullHeight){
if (this.viewConfig.lazyLoadingForFullHeight && this.viewConfig.fullHeight) {
this.trigger(HostEvent.VisibleEmbedCoordinates, data);
}
Comment on lines +1133 to 1141
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 25, 2026

Choose a reason for hiding this comment

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

medium

The calculateVisibleElementData function is called and its result is computed even when the lazyLoadingForFullHeight and fullHeight conditions are not met. Since calculateVisibleElementData internally calls getBoundingClientRect, which can trigger a browser reflow/layout, it is more efficient to perform the configuration check first and only proceed with the calculation if the event is actually going to be triggered.

        if (this.viewConfig.lazyLoadingForFullHeight && this.viewConfig.fullHeight) {
            if (!this.iFrame?.getBoundingClientRect) {
                return;
            }

            const data = calculateVisibleElementData(this.iFrame);
            this.trigger(HostEvent.VisibleEmbedCoordinates, data);
        }

}
Expand All @@ -1145,6 +1149,11 @@
*/
private requestVisibleEmbedCoordinatesHandler = (data: MessagePayload, responder: any) => {
logger.info('Sending RequestVisibleEmbedCoordinates', data);

if (!this.iFrame?.getBoundingClientRect) {
return;
}

const visibleCoordinatesData = calculateVisibleElementData(this.iFrame);
responder({ type: EmbedEvent.RequestVisibleEmbedCoordinates, data: visibleCoordinatesData });
}
Expand Down Expand Up @@ -1177,7 +1186,7 @@
private embedIframeCenter = (data: MessagePayload, responder: any) => {
const obj = this.getIframeCenter();
responder({ type: EmbedEvent.EmbedIframeCenter, data: obj });
};

Check warning on line 1189 in src/embed/app.ts

View workflow job for this annotation

GitHub Actions / build 

Comments may not exceed 80 characters

private setIframeHeightForNonEmbedLiveboard = (data: MessagePayload) => {
const { height: frameHeight } = this.viewConfig.frameParams || {};
Expand Down Expand Up @@ -1304,7 +1313,7 @@
private unregisterLazyLoadEvents() {
if (this.viewConfig.fullHeight && this.viewConfig.lazyLoadingForFullHeight) {
window.removeEventListener('resize', this.sendFullHeightLazyLoadData);
window.removeEventListener('scroll', this.sendFullHeightLazyLoadData);
window.removeEventListener('scroll', this.sendFullHeightLazyLoadData, true);
}
}

Expand Down
43 changes: 42 additions & 1 deletion src/embed/liveboard.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1850,11 +1850,33 @@ describe('Liveboard/viz embed tests', () => {
liveboardEmbed.destroy();

expect(removeEventListenerSpy).toHaveBeenCalledWith('resize', expect.anything());
expect(removeEventListenerSpy).toHaveBeenCalledWith('scroll', expect.anything());
expect(removeEventListenerSpy).toHaveBeenCalledWith('scroll', expect.anything(), true);

removeEventListenerSpy.mockRestore();
});

test('should skip lazy load data when iframe is unavailable', async () => {
const liveboardEmbed = new LiveboardEmbed(getRootEl(), {
...defaultViewConfig,
liveboardId,
fullHeight: true,
lazyLoadingForFullHeight: true,
lazyLoadingMargin: '10px',
} as LiveboardViewConfig);

const mockTrigger = jest.spyOn(liveboardEmbed, 'trigger');

await liveboardEmbed.render();

(liveboardEmbed as any).iFrame = undefined;
(liveboardEmbed as any).sendFullHeightLazyLoadData();

expect(mockTrigger).not.toHaveBeenCalledWith(
HostEvent.VisibleEmbedCoordinates,
expect.anything()
);
});

test('should handle RequestVisibleEmbedCoordinates event and respond with correct data', async () => {
// Mock the iframe element
mockIFrame.getBoundingClientRect = jest.fn().mockReturnValue({
Expand Down Expand Up @@ -1897,6 +1919,25 @@ describe('Liveboard/viz embed tests', () => {
},
});
});

test('should not respond to RequestVisibleEmbedCoordinates when iframe is unavailable', async () => {
const liveboardEmbed = new LiveboardEmbed(getRootEl(), {
...defaultViewConfig,
liveboardId,
fullHeight: true,
lazyLoadingForFullHeight: true,
lazyLoadingMargin: '10px',
} as LiveboardViewConfig);

await liveboardEmbed.render();

const mockResponder = jest.fn();

(liveboardEmbed as any).iFrame = undefined;
(liveboardEmbed as any).requestVisibleEmbedCoordinatesHandler({}, mockResponder);

expect(mockResponder).not.toHaveBeenCalled();
});
});

describe('Host events for liveborad', () => {
Expand Down
13 changes: 11 additions & 2 deletions src/embed/liveboard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,9 +825,13 @@ export class LiveboardEmbed extends V1Embed {
}

private sendFullHeightLazyLoadData = () => {
if (!this.iFrame?.getBoundingClientRect) {
return;
}

const data = calculateVisibleElementData(this.iFrame);
// this should be fired only if the lazyLoadingForFullHeight and fullHeight are true
if(this.viewConfig.lazyLoadingForFullHeight && this.viewConfig.fullHeight){
if (this.viewConfig.lazyLoadingForFullHeight && this.viewConfig.fullHeight) {
this.trigger(HostEvent.VisibleEmbedCoordinates, data);
}
Comment on lines +828 to 836
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 25, 2026

Choose a reason for hiding this comment

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

medium

The calculateVisibleElementData function is called and its result is computed even when the lazyLoadingForFullHeight and fullHeight conditions are not met. Since calculateVisibleElementData internally calls getBoundingClientRect, which can trigger a browser reflow/layout, it is more efficient to perform the configuration check first and only proceed with the calculation if the event is actually going to be triggered.

        if (this.viewConfig.lazyLoadingForFullHeight && this.viewConfig.fullHeight) {
            if (!this.iFrame?.getBoundingClientRect) {
                return;
            }

            const data = calculateVisibleElementData(this.iFrame);
            this.trigger(HostEvent.VisibleEmbedCoordinates, data);
        }

};
Expand All @@ -840,6 +844,11 @@ export class LiveboardEmbed extends V1Embed {
*/
private requestVisibleEmbedCoordinatesHandler = (data: MessagePayload, responder: any) => {
logger.info('Sending RequestVisibleEmbedCoordinates', data);

if (!this.iFrame?.getBoundingClientRect) {
return;
}

const visibleCoordinatesData = calculateVisibleElementData(this.iFrame);
responder({ type: EmbedEvent.RequestVisibleEmbedCoordinates, data: visibleCoordinatesData });
}
Expand Down Expand Up @@ -1033,7 +1042,7 @@ export class LiveboardEmbed extends V1Embed {
private unregisterLazyLoadEvents() {
if (this.viewConfig.fullHeight && this.viewConfig.lazyLoadingForFullHeight) {
window.removeEventListener('resize', this.sendFullHeightLazyLoadData);
window.removeEventListener('scroll', this.sendFullHeightLazyLoadData);
window.removeEventListener('scroll', this.sendFullHeightLazyLoadData, true);
}
}

Expand Down
Loading