diff --git a/src/courseware/course/sequence/Unit.jsx b/src/courseware/course/sequence/Unit.jsx index 2e00a576..ce8e7626 100644 --- a/src/courseware/course/sequence/Unit.jsx +++ b/src/courseware/course/sequence/Unit.jsx @@ -1,8 +1,8 @@ import React, { Suspense, - useRef, useEffect, useState, + useLayoutEffect, } from 'react'; import PropTypes from 'prop-types'; import { getConfig } from '@edx/frontend-platform'; @@ -14,13 +14,44 @@ import PageLoading from '../../../PageLoading'; const LockPaywall = React.lazy(() => import('./lock-paywall')); +/** + * We discovered an error in Firefox where - upon iframe load - React would cease to call any + * useEffect hooks until the user interacts with the page again. This is particularly confusing + * when navigating between sequences, as the UI partially updates leaving the user in a nebulous + * state. + * + * We were able to solve this error by using a layout effect to update some component state, which + * executes synchronously on render. Somehow this forces React to continue it's lifecycle + * immediately, rather than waiting for user interaction. This layout effect could be anywhere in + * the parent tree, as far as we can tell - we chose to add a conspicuously 'load bearing' (that's + * a joke) one here so it wouldn't be accidentally removed elsewhere. + * + * If we remove this hook when one of these happens: + * 1. React figures out that there's an issue here and fixes a bug. + * 2. We cease to use an iframe for unit rendering. + * 3. Firefox figures out that there's an issue in their iframe loading and fixes a bug. + * 4. We stop supporting Firefox. + * 5. An enterprising engineer decides to create a repo that reproduces the problem, submits it to + * Firefox/React for review, and they kindly help us figure out what in the world is happening + * so we can fix it. + * + * This hook depends on the unit id just to make sure it re-evaluates whenever the ID changes. If + * we change whether or not the Unit component is re-mounted when the unit ID changes, this may + * become important, as this hook will otherwise only evaluate the useLayoutEffect once. + */ +function useLoadBearingHook(id) { + const setValue = useState(0)[1]; + useLayoutEffect(() => { + setValue(currentValue => currentValue + 1); + }, [id]); +} + function Unit({ courseId, onLoaded, id, intl, }) { - const iframeRef = useRef(null); const iframeUrl = `${getConfig().LMS_BASE_URL}/xblock/${id}?show_title=0&show_bookmark_button=0`; const [iframeHeight, setIframeHeight] = useState(0); @@ -33,25 +64,29 @@ function Unit({ enrollmentMode, } = course; - useEffect(() => { - global.onmessage = (event) => { - const { type, payload } = event.data; + // Do not remove this hook. See function description. + useLoadBearingHook(id); - console.log('MFE App', 'onmessage', id, iframeUrl, event.source, type, payload, event); - if (type === 'plugin.resize') { - setIframeHeight(payload.height); - console.log('MFE App', 'plugin.resize', hasLoaded, iframeHeight, payload.height); - if (!hasLoaded && iframeHeight === 0 && payload.height > 0) { - console.log('MFE App', 'hasLoaded', true); - setHasLoaded(true); - if (onLoaded) { - console.log('MFE App', 'fire onLoaded handler'); - onLoaded(); + useEffect(() => { + if (!global.onmessage) { + global.onmessage = (event) => { + const { type, payload } = event.data; + + if (type === 'plugin.resize') { + setIframeHeight(payload.height); + if (!hasLoaded && iframeHeight === 0 && payload.height > 0) { + setHasLoaded(true); + if (onLoaded) { + onLoaded(); + } } } - } + }; + } + return () => { + global.onmessage = null; }; - }, []); + }, [id]); return (