Skip to content

Commit

Permalink
Merge branch 'rpenido/fal-3532-import-taxonomy' into rpenido/fal-3539…
Browse files Browse the repository at this point in the history
…-refined-ux-update-a-taxonomy-by-downloading-and-uploading
  • Loading branch information
rpenido authored Dec 6, 2023
2 parents 1a94683 + f318dfc commit 58964a2
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 88 deletions.
17 changes: 0 additions & 17 deletions src/taxonomy/taxonomy-card/TaxonomyCard.scss
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,4 @@
text-overflow: ellipsis;
white-space: nowrap;
}

.taxonomy-menu-item:focus {
/**
* There is a bug in the menu that auto focus the first item.
* We convert the focus style to a normal style.
*/
background-color: white !important;
font-weight: normal !important;
}

.taxonomy-menu-item:focus:hover {
/**
* Check the previous block about the focus.
* This enable a normal hover to focused items.
*/
background-color: $light-500 !important;
}
}
26 changes: 7 additions & 19 deletions src/taxonomy/taxonomy-card/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,25 +70,13 @@ const TaxonomyCard = ({ className, original }) => {
} = original;

const intl = useIntl();

const getHeaderActions = () => {
if (systemDefined) {
// We don't show the export menu, because the system-taxonomies
// can't be exported. The API returns and error.
// The entire menu has been hidden because currently only
// the export menu exists.
//
// TODO When adding more menus, change this logic to hide only the export menu.
return undefined;
}

return (
<TaxonomyMenu
taxonomy={original}
iconMenu
/>
);
};

Check failure on line 73 in src/taxonomy/taxonomy-card/index.jsx

View workflow job for this annotation

GitHub Actions / tests

Trailing spaces not allowed
const getHeaderActions = () => (
<TaxonomyMenu
taxonomy={original}
iconMenu
/>
);

return (
<Card
Expand Down
8 changes: 0 additions & 8 deletions src/taxonomy/taxonomy-detail/TaxonomyDetailPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,6 @@ const TaxonomyDetailPage = () => {
const getHeaderActions = () => (
<TaxonomyMenu
taxonomy={taxonomy}
disabled={
// We don't show the export menu, because the system-taxonomies
// can't be exported. The API returns and error.
// The entire menu has been disabled because currently only
// the export menu exists.
// ToDo: When adding more menus, change this logic to hide only the export menu.
taxonomy.systemDefined
}
/>
);

Expand Down
84 changes: 45 additions & 39 deletions src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,48 +9,49 @@ import {
} from '@edx/paragon';
import { MoreVert } from '@edx/paragon/icons';
import PropTypes from 'prop-types';
import _ from 'lodash';

import ExportModal from '../export-modal';
import { ImportTagsWizard } from '../import-tags';

Check failure on line 15 in src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx

View workflow job for this annotation

GitHub Actions / tests

'ImportTagsWizard' is defined but never used
import messages from './messages';

const TaxonomyMenu = ({
taxonomy, iconMenu, disabled,
taxonomy, iconMenu,
}) => {
const intl = useIntl();

const [isExportModalOpen, exportModalOpen, exportModalClose] = useToggle(false);
const [isImportModalOpen, importModalOpen, importModalClose] = useToggle(false);

Check failure on line 24 in src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx

View workflow job for this annotation

GitHub Actions / tests

'isImportModalOpen' is assigned a value but never used

Check failure on line 24 in src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx

View workflow job for this annotation

GitHub Actions / tests

'importModalClose' is assigned a value but never used

const menuItemActions = {
import: importModalOpen,
export: exportModalOpen,
};
const getTaxonomyMenuItems = () => {
let menuItems = {
import: {
title: intl.formatMessage(messages.importMenu),
action: importModalOpen,
// Hide import menu item if taxonomy is system defined or allows free text
hide: taxonomy.systemDefined || taxonomy.allowFreeText,
},
export: {
title: intl.formatMessage(messages.exportMenu),
action: exportModalOpen,
},
};

// Remove hidden menu items
menuItems = _.pickBy(menuItems, (value) => !value.hide);

const onClickMenuItem = (e, menuName) => {
e.preventDefault();
menuItemActions[menuName]?.();
return menuItems;
};

const renderModals = () => (
<>
{isExportModalOpen && (
<ExportModal
taxonomyId={taxonomy.id}
isOpen={isExportModalOpen}
onClose={exportModalClose}
taxonomyName={taxonomy.name}
/>
)}
{isImportModalOpen && (
<ImportTagsWizard
taxonomy={taxonomy}
isOpen={isImportModalOpen}
close={importModalClose}
/>
)}
const menuItems = getTaxonomyMenuItems();

</>
const renderModals = () => isExportModalOpen && (
<ExportModal
isOpen={isExportModalOpen}
onClose={exportModalClose}
taxonomyId={taxonomy.id}
taxonomyName={taxonomy.name}
/>
);

return (
Expand All @@ -62,17 +63,25 @@ const TaxonomyMenu = ({
variant="primary"
alt={intl.formatMessage(messages.actionsButtonAlt, { name: taxonomy.name })}
data-testid="taxonomy-menu-button"
disabled={disabled}
disabled={menuItems.length === 0}
>
{intl.formatMessage(messages.actionsButtonLabel)}
</Dropdown.Toggle>
<Dropdown.Menu data-testid="taxonomy-menu">
<Dropdown.Item data-testid="taxonomy-menu-import" onClick={(e) => onClickMenuItem(e, 'import')}>
{intl.formatMessage(messages.importMenu)}
</Dropdown.Item>
<Dropdown.Item data-testid="taxonomy-menu-export" onClick={(e) => onClickMenuItem(e, 'export')}>
{intl.formatMessage(messages.exportMenu)}
</Dropdown.Item>
{Object.keys(menuItems).map((key) => (
<Dropdown.Item
key={key}
data-testid={`taxonomy-menu-${key}`}
onClick={
(e) => {
e.preventDefault();
menuItems[key].action?.();
}
}
>
{menuItems[key].title}
</Dropdown.Item>
))}
</Dropdown.Menu>
{renderModals()}
</Dropdown>
Expand All @@ -83,13 +92,10 @@ TaxonomyMenu.propTypes = {
taxonomy: PropTypes.shape({
id: PropTypes.number.isRequired,
name: PropTypes.string.isRequired,
systemDefined: PropTypes.bool.isRequired,
allowFreeText: PropTypes.bool.isRequired,
}).isRequired,
iconMenu: PropTypes.bool.isRequired,
disabled: PropTypes.bool,
};

TaxonomyMenu.defaultProps = {
disabled: false,
};

export default TaxonomyMenu;
export default TaxonomyMenu;

Check failure on line 101 in src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx

View workflow job for this annotation

GitHub Actions / tests

Newline required at end of file but not found
53 changes: 48 additions & 5 deletions src/taxonomy/taxonomy-menu/TaxonomyMenu.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,34 @@ jest.mock('../data/api', () => ({
}));

const TaxonomyMenuComponent = ({
systemDefined,
allowFreeText,
iconMenu,
disabled,
}) => (
<AppProvider store={store}>
<IntlProvider locale="en" messages={{}}>
<TaxonomyMenu id={taxonomyId} name={taxonomyName} iconMenu={iconMenu} disabled={disabled} />
<TaxonomyMenu
taxonomy={{
id: taxonomyId,
name: taxonomyName,
systemDefined,
allowFreeText,
}}
iconMenu={iconMenu}
/>
</IntlProvider>
</AppProvider>
);

TaxonomyMenuComponent.propTypes = {
iconMenu: PropTypes.bool.isRequired,
disabled: PropTypes.bool,
systemDefined: PropTypes.bool,
allowFreeText: PropTypes.bool,
};

TaxonomyMenuComponent.defaultProps = {
disabled: false,
systemDefined: false,
allowFreeText: false,
};

describe('<TaxonomyMenu />', async () => {
Expand Down Expand Up @@ -78,6 +89,38 @@ describe('<TaxonomyMenu />', async () => {
expect(getByTestId('taxonomy-menu-button')).toBeVisible();
});

test('doesnt show systemDefined taxonomies disabled menus', () => {
const { getByTestId } = render(<TaxonomyMenuComponent iconMenu={iconMenu} systemDefined />);

// Menu closed/doesn't exist yet
expect(() => getByTestId('taxonomy-menu')).toThrow();

// Click on the menu button to open
fireEvent.click(getByTestId('taxonomy-menu-button'));

// Menu opened
expect(getByTestId('taxonomy-menu')).toBeVisible();

// Check that the import menu is not show
expect(() => getByTestId('taxonomy-menu-import')).toThrow();
});

test('doesnt show freeText taxonomies disabled menus', () => {
const { getByTestId } = render(<TaxonomyMenuComponent iconMenu={iconMenu} allowFreeText />);

// Menu closed/doesn't exist yet
expect(() => getByTestId('taxonomy-menu')).toThrow();

// Click on the menu button to open
fireEvent.click(getByTestId('taxonomy-menu-button'));

// Menu opened
expect(getByTestId('taxonomy-menu')).toBeVisible();

// Check that the import menu is not show
expect(() => getByTestId('taxonomy-menu-import')).toThrow();
});

test('should open export modal on export menu click', () => {
const { getByTestId, getByText } = render(<TaxonomyMenuComponent iconMenu={iconMenu} />);

Expand Down Expand Up @@ -125,4 +168,4 @@ describe('<TaxonomyMenu />', async () => {
expect(getTaxonomyExportFile).toHaveBeenCalledWith(taxonomyId, 'json');
});
});
});
});

Check failure on line 171 in src/taxonomy/taxonomy-menu/TaxonomyMenu.test.jsx

View workflow job for this annotation

GitHub Actions / tests

Newline required at end of file but not found

0 comments on commit 58964a2

Please sign in to comment.