fix: unit preview in course outline sidebar (#2940)

This commit is contained in:
Navin Karkera
2026-03-14 00:12:39 +05:30
committed by GitHub
parent 24e1c73f6b
commit df79861685
3 changed files with 31 additions and 29 deletions

View File

@@ -94,9 +94,11 @@ export const UnitSidebarProvider = ({
);
};
export function useUnitSidebarContext(): UnitSidebarContextData {
export function useUnitSidebarContext(raiseError?: true): UnitSidebarContextData;
export function useUnitSidebarContext(raiseError?: boolean): UnitSidebarContextData | undefined;
export function useUnitSidebarContext(raiseError: boolean = true): UnitSidebarContextData | undefined {
const ctx = useContext(UnitSidebarContext);
if (ctx === undefined) {
if (ctx === undefined && raiseError) {
/* istanbul ignore next */
throw new Error('useUnitSidebarContext() was used in a component without a <UnitSidebarProvider> ancestor.');
}

View File

@@ -58,7 +58,7 @@ const XBlockContainerIframe: FC<XBlockContainerIframeProps> = ({
const {
setCurrentPageKey,
setSelectedComponentId,
} = useUnitSidebarContext();
} = useUnitSidebarContext(!readonly) || {};
// Useful to reload iframe
const [iframeKey, setIframeKey] = useState(0);
@@ -138,7 +138,7 @@ const XBlockContainerIframe: FC<XBlockContainerIframeProps> = ({
const onDeleteSubmit = async () => {
if (deleteXBlockId) {
await unitXBlockActions.handleDelete(deleteXBlockId);
setSelectedComponentId(undefined);
setSelectedComponentId?.(undefined);
closeDeleteModal();
}
};
@@ -192,7 +192,7 @@ const XBlockContainerIframe: FC<XBlockContainerIframeProps> = ({
const handleOpenManageTagsModal = (id: string) => {
if (isUnitPageNewDesignEnabled()) {
setCurrentPageKey('align', id);
setCurrentPageKey?.('align', id);
sendMessageToIframe(messageTypes.selectXblock, { locator: id });
} else {
// Legacy manage tags modal
@@ -219,7 +219,7 @@ const XBlockContainerIframe: FC<XBlockContainerIframeProps> = ({
};
const handleXBlockSelected = (id) => {
setCurrentPageKey('info', id);
setCurrentPageKey?.('info', id);
};
const messageHandlers = useMessageHandlers({

View File

@@ -29,20 +29,20 @@ describe('useWaffleFlags', () => {
axiosMock.onGet(getApiWaffleFlagsUrl()).reply(() => promise);
render(<FlagComponent />);
expect(screen.getByLabelText('isLoading')).toHaveTextContent('loading');
expect(screen.getByLabelText('isError')).toHaveTextContent('false');
expect(await screen.findByLabelText('isLoading')).toHaveTextContent('loading');
expect(await screen.findByLabelText('isError')).toHaveTextContent('false');
// The default should be enabled, even before we hear back from the server:
expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled');
expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled');
// Then, the server responds with a new value:
resolveResponse([200, { useNewCourseOutlinePage: false }]);
// Now, we're no longer loading and we have the new value:
await waitFor(() => {
expect(screen.getByLabelText('isLoading')).toHaveTextContent('false');
await waitFor(async () => {
expect(await screen.findByLabelText('isLoading')).toHaveTextContent('false');
});
expect(screen.getByLabelText('isError')).toHaveTextContent('false');
expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled');
expect(await screen.findByLabelText('isError')).toHaveTextContent('false');
expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled');
});
it('uses the default values if there\'s an error', async () => {
@@ -53,20 +53,20 @@ describe('useWaffleFlags', () => {
axiosMock.onGet(getApiWaffleFlagsUrl()).reply(() => promise);
render(<FlagComponent />);
expect(screen.getByLabelText('isLoading')).toHaveTextContent('loading');
expect(screen.getByLabelText('isError')).toHaveTextContent('false');
expect(await screen.findByLabelText('isLoading')).toHaveTextContent('loading');
expect(await screen.findByLabelText('isError')).toHaveTextContent('false');
// The default should be enabled, even before we hear back from the server:
expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled');
expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled');
// Then, the server responds with an error
resolveResponse([500, {}]);
// Now, we're no longer loading, we have an error state, and we still have the default value:
await waitFor(() => {
expect(screen.getByLabelText('isLoading')).toHaveTextContent('false');
await waitFor(async () => {
expect(await screen.findByLabelText('isLoading')).toHaveTextContent('false');
});
expect(screen.getByLabelText('isError')).toHaveTextContent('error');
expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled');
expect(await screen.findByLabelText('isError')).toHaveTextContent('error');
expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled');
});
it('uses the global flag values while loading the course-specific flags', async () => {
@@ -81,9 +81,9 @@ describe('useWaffleFlags', () => {
// Check the global flag:
render(<FlagComponent />);
await waitFor(() => {
await waitFor(async () => {
// Once it loads the flags from the server, the global 'false' value will override the default 'true':
expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled');
expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled');
});
// Now check the course-specific flag:
@@ -91,16 +91,16 @@ describe('useWaffleFlags', () => {
render(<FlagComponent courseId={courseId} />);
// Now, the course-specific value is loading but in the meantime we use the global default:
expect(screen.getByLabelText('isLoading')).toHaveTextContent('loading');
expect(screen.getByLabelText('isError')).toHaveTextContent('false');
expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled');
expect(await screen.findByLabelText('isLoading')).toHaveTextContent('loading');
expect(await screen.findByLabelText('isError')).toHaveTextContent('false');
expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('disabled');
// Now the server responds: the course-specific flag is ON:
resolveResponse([200, { useNewCourseOutlinePage: true }]);
await waitFor(() => {
expect(screen.getByLabelText('isLoading')).toHaveTextContent('false');
await waitFor(async () => {
expect(await screen.findByLabelText('isLoading')).toHaveTextContent('false');
});
expect(screen.getByLabelText('isError')).toHaveTextContent('false');
expect(screen.getByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled');
expect(await screen.findByLabelText('isError')).toHaveTextContent('false');
expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled');
});
});