Skip to content

Commit f01d40b

Browse files
committed
fix: ensure iframe visibility tracking is triggered on load
The previous implementation had a race condition that sometimes prevented XBlocks from being marked as viewed. Users had to scroll or resize the window to trigger visibility tracking instead of having it happen once content loads.
1 parent 78dd94a commit f01d40b

File tree

2 files changed

+37
-30
lines changed

2 files changed

+37
-30
lines changed

src/courseware/course/sequence/Unit/hooks/useIFrameBehavior.js

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,23 @@ const useIFrameBehavior = ({
8686
useEventListener('message', receiveMessage);
8787

8888
// Send visibility status to the iframe. It's used to mark XBlocks as viewed.
89+
const updateIframeVisibility = () => {
90+
const iframeElement = document.getElementById(elementId);
91+
const rect = iframeElement.getBoundingClientRect();
92+
const visibleInfo = {
93+
type: 'unit.visibilityStatus',
94+
data: {
95+
topPosition: rect.top,
96+
viewportHeight: window.innerHeight,
97+
},
98+
};
99+
iframeElement.contentWindow.postMessage(
100+
visibleInfo,
101+
`${getConfig().LMS_BASE_URL}`,
102+
);
103+
};
104+
105+
// Set up visibility tracking event listeners.
89106
React.useEffect(() => {
90107
if (!hasLoaded) {
91108
return undefined;
@@ -96,27 +113,9 @@ const useIFrameBehavior = ({
96113
return undefined;
97114
}
98115

99-
const updateIframeVisibility = () => {
100-
const rect = iframeElement.getBoundingClientRect();
101-
const visibleInfo = {
102-
type: 'unit.visibilityStatus',
103-
data: {
104-
topPosition: rect.top,
105-
viewportHeight: window.innerHeight,
106-
},
107-
};
108-
iframeElement.contentWindow.postMessage(
109-
visibleInfo,
110-
`${getConfig().LMS_BASE_URL}`,
111-
);
112-
};
113-
114116
// Throttle the update function to prevent it from sending too many messages to the iframe.
115117
const throttledUpdateVisibility = throttle(updateIframeVisibility, 100);
116118

117-
// Update the visibility of the iframe in case the element is already visible.
118-
updateIframeVisibility();
119-
120119
// Add event listeners to update the visibility of the iframe when the window is scrolled or resized.
121120
window.addEventListener('scroll', throttledUpdateVisibility);
122121
window.addEventListener('resize', throttledUpdateVisibility);
@@ -147,6 +146,9 @@ const useIFrameBehavior = ({
147146
dispatch(processEvent(e.data, fetchCourse));
148147
}
149148
};
149+
150+
// Update the visibility of the iframe in case the element is already visible.
151+
updateIframeVisibility();
150152
};
151153

152154
return {

src/courseware/course/sequence/Unit/hooks/useIFrameBehavior.test.js

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ describe('useIFrameBehavior hook', () => {
275275
});
276276
});
277277
describe('visibility tracking', () => {
278-
it('sets up visibility tracking after iframe has loaded', () => {
278+
it('sets up visibility tracking after iframe loads', () => {
279279
state.mockVals({ ...defaultStateVals, hasLoaded: true });
280280
useIFrameBehavior(props);
281281

@@ -286,17 +286,8 @@ describe('useIFrameBehavior hook', () => {
286286
expect(global.window.addEventListener).toHaveBeenCalledTimes(2);
287287
expect(global.window.addEventListener).toHaveBeenCalledWith('scroll', expect.any(Function));
288288
expect(global.window.addEventListener).toHaveBeenCalledWith('resize', expect.any(Function));
289-
// Initial visibility update.
290-
expect(postMessage).toHaveBeenCalledWith(
291-
{
292-
type: 'unit.visibilityStatus',
293-
data: {
294-
topPosition: 100,
295-
viewportHeight: 800,
296-
},
297-
},
298-
config.LMS_BASE_URL,
299-
);
289+
// Initial visibility update is handled by the `handleIFrameLoad` method.
290+
expect(postMessage).not.toHaveBeenCalled();
300291
});
301292
it('does not set up visibility tracking before iframe has loaded', () => {
302293
state.mockVals({ ...defaultStateVals, hasLoaded: false });
@@ -345,6 +336,20 @@ describe('useIFrameBehavior hook', () => {
345336
window.onmessage(event);
346337
expect(dispatch).toHaveBeenCalledWith(processEvent(event.data, fetchCourse));
347338
});
339+
it('updates initial iframe visibility on load', () => {
340+
hook = useIFrameBehavior(props);
341+
hook.handleIFrameLoad();
342+
expect(postMessage).toHaveBeenCalledWith(
343+
{
344+
type: 'unit.visibilityStatus',
345+
data: {
346+
topPosition: 100,
347+
viewportHeight: 800,
348+
},
349+
},
350+
config.LMS_BASE_URL,
351+
);
352+
});
348353
});
349354
it('forwards handleIframeLoad, showError, and hasLoaded from state fields', () => {
350355
state.mockVals(stateVals);

0 commit comments

Comments
 (0)