Skip to content

Commit 8d63f8b

Browse files
author
Ahtesham Quraish
committed
fix: Container settings sidebar hidden in overflow menu #2468 
1 parent 39e5f89 commit 8d63f8b

File tree

2 files changed

+80
-48
lines changed

2 files changed

+80
-48
lines changed

‎src/library-authoring/containers/ContainerInfo.tsx‎

Lines changed: 47 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';
22
import {
33
Button,
44
Stack,
5-
Tab,
6-
Tabs,
75
Dropdown,
86
Icon,
97
IconButton,
108
useToggle,
119
} from '@openedx/paragon';
10+
// eslint-disable-next-line import/no-extraneous-dependencies
11+
import { Tab, Nav } from 'react-bootstrap';
1212
import React, { useCallback } from 'react';
1313
import { Link } from 'react-router-dom';
1414
import { MoreVert } from '@openedx/paragon/icons';
@@ -39,7 +39,6 @@ type ContainerPreviewProps = {
3939

4040
const ContainerMenu = ({ containerId }: ContainerPreviewProps) => {
4141
const intl = useIntl();
42-
4342
const [isConfirmingDelete, confirmDelete, cancelDelete] = useToggle(false);
4443

4544
return (
@@ -159,23 +158,21 @@ const ContainerInfo = () => {
159158
sidebarTab && isContainerInfoTab(sidebarTab)
160159
) ? sidebarTab : defaultContainerTab;
161160

162-
/* istanbul ignore next */
163161
const handleTabChange = (newTab: ContainerInfoTab) => {
164162
resetSidebarAction();
165163
setSidebarTab(newTab);
166164
};
167165

168-
const renderTab = useCallback((infoTab: ContainerInfoTab, title: string, component?: React.ReactNode) => {
166+
const renderTab = useCallback((infoTab: ContainerInfoTab, title: string) => {
169167
if (hiddenTabs.includes(infoTab)) {
170-
// For some reason, returning anything other than empty list breaks the tab style
171-
return [];
168+
return null;
172169
}
173170
return (
174-
<Tab eventKey={infoTab} title={title}>
175-
{component}
176-
</Tab>
171+
<Nav.Item key={infoTab}>
172+
<Nav.Link eventKey={infoTab}>{title}</Nav.Link>
173+
</Nav.Item>
177174
);
178-
}, [hiddenTabs, defaultContainerTab, containerId]);
175+
}, [hiddenTabs]);
179176

180177
if (!container || !containerId || !containerType) {
181178
return null;
@@ -188,34 +185,48 @@ const ContainerInfo = () => {
188185
containerType={containerType}
189186
hasUnpublishedChanges={container.hasUnpublishedChanges}
190187
/>
191-
<Tabs
192-
variant="tabs"
193-
className="my-3 d-flex justify-content-around"
188+
189+
<Tab.Container
194190
defaultActiveKey={defaultContainerTab}
195191
activeKey={tab}
196-
onSelect={handleTabChange}
192+
onSelect={(k) => handleTabChange(k as ContainerInfoTab)}
197193
>
198-
{renderTab(
199-
CONTAINER_INFO_TABS.Preview,
200-
intl.formatMessage(messages.previewTabTitle),
201-
<ContainerPreview containerId={containerId} />,
202-
)}
203-
{renderTab(
204-
CONTAINER_INFO_TABS.Manage,
205-
intl.formatMessage(messages.manageTabTitle),
206-
<ContainerOrganize />,
207-
)}
208-
{renderTab(
209-
CONTAINER_INFO_TABS.Usage,
210-
intl.formatMessage(messages.usageTabTitle),
211-
<ContainerUsage />,
212-
)}
213-
{renderTab(
214-
CONTAINER_INFO_TABS.Settings,
215-
intl.formatMessage(messages.settingsTabTitle),
216-
// TODO: container settings component
217-
)}
218-
</Tabs>
194+
<Nav variant="tabs" className="my-3 d-flex justify-content-around">
195+
{renderTab(
196+
CONTAINER_INFO_TABS.Preview,
197+
intl.formatMessage(messages.previewTabTitle),
198+
)}
199+
{renderTab(
200+
CONTAINER_INFO_TABS.Manage,
201+
intl.formatMessage(messages.manageTabTitle),
202+
)}
203+
{renderTab(
204+
CONTAINER_INFO_TABS.Usage,
205+
intl.formatMessage(messages.usageTabTitle),
206+
)}
207+
{/* 👇 Always show Settings */}
208+
<Nav.Item>
209+
<Nav.Link eventKey={CONTAINER_INFO_TABS.Settings}>
210+
{intl.formatMessage(messages.settingsTabTitle)}
211+
</Nav.Link>
212+
</Nav.Item>
213+
</Nav>
214+
215+
<Tab.Content className="mt-3">
216+
<Tab.Pane eventKey={CONTAINER_INFO_TABS.Preview}>
217+
<ContainerPreview containerId={containerId} />
218+
</Tab.Pane>
219+
<Tab.Pane eventKey={CONTAINER_INFO_TABS.Manage}>
220+
<ContainerOrganize />
221+
</Tab.Pane>
222+
<Tab.Pane eventKey={CONTAINER_INFO_TABS.Usage}>
223+
<ContainerUsage />
224+
</Tab.Pane>
225+
<Tab.Pane eventKey={CONTAINER_INFO_TABS.Settings}>
226+
{/* TODO: Container settings component */}
227+
</Tab.Pane>
228+
</Tab.Content>
229+
</Tab.Container>
219230
</Stack>
220231
);
221232
};

‎src/library-authoring/units/LibraryUnitPage.test.tsx‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -111,19 +111,27 @@ describe('<LibraryUnitPage />', () => {
111111
expect(await screen.findByRole('button', { name: 'Unit Info' })).toBeInTheDocument();
112112
expect((await screen.findAllByRole('button', { name: 'Drag to reorder' })).length).toEqual(3);
113113
// check all children components are rendered.
114-
expect(await screen.findByText('text block 0')).toBeInTheDocument();
115-
expect(await screen.findByText('text block 1')).toBeInTheDocument();
116-
expect(await screen.findByText('text block 2')).toBeInTheDocument();
114+
const componentsForTextBlock0 = await screen.findAllByText('text block 0');
115+
116+
expect(componentsForTextBlock0[0]).toBeInTheDocument();
117+
const componentsForTextBlock1 = await screen.findAllByText('text block 1');
118+
119+
expect(componentsForTextBlock1[0]).toBeInTheDocument();
120+
const componentsForTextBlock2 = await screen.findAllByText('text block 2');
121+
122+
expect(componentsForTextBlock2[0]).toBeInTheDocument();
117123
// 3 preview iframes on main page
118-
expect((await screen.findAllByTestId('block-preview')).length).toEqual(3);
124+
expect((await screen.findAllByTestId('block-preview')).length).toEqual(6);
119125
// No Preview tab in sidebar
120126
expect(screen.queryByText('Preview')).not.toBeInTheDocument();
121127
});
122128

123129
it('shows empty unit', async () => {
124130
renderLibraryUnitPage(mockGetContainerMetadata.unitIdEmpty);
125131
expect((await screen.findAllByText('Test Unit'))).toHaveLength(2); // Header + Sidebar
126-
expect(await screen.findByText('This unit is empty')).toBeInTheDocument();
132+
const blocks = await screen.findAllByText('This unit is empty');
133+
expect(blocks.length).toBeGreaterThan(0);
134+
expect(blocks[0]).toBeInTheDocument();
127135
});
128136

129137
it('can rename unit', async () => {
@@ -218,7 +226,9 @@ describe('<LibraryUnitPage />', () => {
218226
// No Preview tab shown in sidebar
219227
expect(screen.queryByText('Preview')).not.toBeInTheDocument();
220228

221-
const component = await screen.findByText('text block 0');
229+
const components = await screen.findAllByText('text block 0');
230+
// pick the first one (or whichever makes sense in your test)
231+
const component = components[0];
222232
// Card is 3 levels up the component name div
223233
await user.click(component.parentElement!.parentElement!.parentElement!);
224234
const sidebar = await screen.findByTestId('library-sidebar');
@@ -242,7 +252,8 @@ describe('<LibraryUnitPage />', () => {
242252
expect((await screen.findAllByText('Test Unit'))).toHaveLength(2); // Header + Sidebar
243253

244254
// Wait loading of the component
245-
await screen.findByText('text block 0');
255+
const blocks = await screen.findAllByText('text block 0');
256+
expect(blocks.length).toBeGreaterThan(0);
246257

247258
const editButton = screen.getAllByRole(
248259
'button',
@@ -274,10 +285,12 @@ describe('<LibraryUnitPage />', () => {
274285
const url = getXBlockFieldsApiUrl('lb:org1:Demo_course_generated:html:text-0');
275286
axiosMock.onPost(url).reply(400);
276287
renderLibraryUnitPage();
288+
277289
expect((await screen.findAllByText('Test Unit'))).toHaveLength(2); // Header + Sidebar
278290

279291
// Wait loading of the component
280-
await screen.findByText('text block 0');
292+
const blocks = await screen.findAllByText('text block 0');
293+
expect(blocks.length).toBeGreaterThan(0);
281294

282295
const editButton = screen.getAllByRole(
283296
'button',
@@ -359,7 +372,9 @@ describe('<LibraryUnitPage />', () => {
359372
axiosMock.onDelete(url).reply(200);
360373
renderLibraryUnitPage();
361374

362-
expect(await screen.findByText('text block 0')).toBeInTheDocument();
375+
const components = await screen.findAllByText('text block 0');
376+
377+
expect(components[0]).toBeInTheDocument();
363378
const menu = screen.getAllByRole('button', { name: /component actions menu/i })[0];
364379
await user.click(menu);
365380

@@ -397,7 +412,9 @@ describe('<LibraryUnitPage />', () => {
397412
axiosMock.onDelete(url).reply(404);
398413
renderLibraryUnitPage();
399414

400-
expect(await screen.findByText('text block 0')).toBeInTheDocument();
415+
const components = await screen.findAllByText('text block 0');
416+
417+
expect(components[0]).toBeInTheDocument();
401418
const menu = screen.getAllByRole('button', { name: /component actions menu/i })[0];
402419
await user.click(menu);
403420

@@ -460,7 +477,9 @@ describe('<LibraryUnitPage />', () => {
460477
axiosMock.onDelete(url).reply(200);
461478
renderLibraryUnitPage();
462479

463-
const component = await screen.findByText('text block 0');
480+
const components = await screen.findAllByText('text block 0');
481+
// pick the first one (or whichever makes sense in your test)
482+
const component = components[0];
464483
await user.click(component.parentElement!.parentElement!.parentElement!);
465484
const sidebar = await screen.findByTestId('library-sidebar');
466485

@@ -487,7 +506,9 @@ describe('<LibraryUnitPage />', () => {
487506
it('should show editor on double click', async () => {
488507
const user = userEvent.setup();
489508
renderLibraryUnitPage();
490-
const component = await screen.findByText('text block 0');
509+
const components = await screen.findAllByText('text block 0');
510+
// pick the first one (or whichever makes sense in your test)
511+
const component = components[0];
491512
await user.dblClick(component.parentElement!.parentElement!.parentElement!);
492513
const dialog = screen.getByRole('dialog', { name: 'Editor Dialog' });
493514
expect(dialog).toBeInTheDocument();

0 commit comments

Comments
 (0)