fix: course updates UI bugs (#606)

* fix: change edit and delete buttons to icons

* fix: padding and button color

* fix: delete buttons and variant

* fix: date error icon color

* fix: page explanation text
This commit is contained in:
Kristin Aoki
2023-09-22 15:51:04 -04:00
committed by GitHub
parent 217b86e616
commit ef9633af35
12 changed files with 75 additions and 73 deletions

View File

@@ -63,7 +63,7 @@ const CourseUpdates = ({ courseId }) => {
return (
<>
<Container size="xl" className="m-4">
<Container size="xl" className="px-4">
<section className="setting-items mb-4 mt-5">
<Layout
lg={[{ span: 12 }]}
@@ -81,7 +81,7 @@ const CourseUpdates = ({ courseId }) => {
instruction={intl.formatMessage(messages.sectionInfo)}
headerActions={(
<Button
variant="outline-primary"
variant="primary"
iconBefore={AddIcon}
size="sm"
onClick={() => handleOpenUpdateForm(REQUEST_TYPES.add_new_update)}

View File

@@ -163,37 +163,39 @@ describe('<CourseUpdates />', () => {
});
it('Add new update form is visible after clicking "New update" button', async () => {
const { getByText, getByRole, getAllByRole } = render(<RootWrapper />);
const { getByText, getByRole, getAllByTestId } = render(<RootWrapper />);
await waitFor(() => {
const editButtons = getAllByRole('button', { name: 'Edit' });
const deleteButtons = getAllByRole('button', { name: 'Delete' });
const editUpdateButtons = getAllByTestId('course-update-edit-button');
const deleteButtons = getAllByTestId('course-update-delete-button');
const editHandoutsButtons = getAllByTestId('course-handouts-edit-button');
const newUpdateButton = getByRole('button', { name: messages.newUpdateButton.defaultMessage });
fireEvent.click(newUpdateButton);
expect(newUpdateButton).toBeDisabled();
editButtons.forEach((button) => expect(button).toBeDisabled());
editUpdateButtons.forEach((button) => expect(button).toBeDisabled());
editHandoutsButtons.forEach((button) => expect(button).toBeDisabled());
deleteButtons.forEach((button) => expect(button).toBeDisabled());
expect(getByText('Add new update')).toBeInTheDocument();
});
});
it('Edit handouts form is visible after clicking "Edit" button', async () => {
const {
getByText, getByRole, getByTestId, getAllByRole,
} = render(<RootWrapper />);
const { getByText, getByRole, getAllByTestId } = render(<RootWrapper />);
await waitFor(() => {
const editHandoutsButton = getByTestId('course-handouts-edit-button');
const editButtons = getAllByRole('button', { name: 'Edit' });
const deleteButtons = getAllByRole('button', { name: 'Delete' });
const editUpdateButtons = getAllByTestId('course-update-edit-button');
const deleteButtons = getAllByTestId('course-update-delete-button');
const editHandoutsButtons = getAllByTestId('course-handouts-edit-button');
const editHandoutsButton = editHandoutsButtons[0];
fireEvent.click(editHandoutsButton);
expect(editHandoutsButton).toBeDisabled();
expect(getByRole('button', { name: messages.newUpdateButton.defaultMessage })).toBeDisabled();
editButtons.forEach((button) => expect(button).toBeDisabled());
editUpdateButtons.forEach((button) => expect(button).toBeDisabled());
editHandoutsButtons.forEach((button) => expect(button).toBeDisabled());
deleteButtons.forEach((button) => expect(button).toBeDisabled());
expect(getByText('Edit handouts')).toBeInTheDocument();
});
@@ -201,18 +203,20 @@ describe('<CourseUpdates />', () => {
it('Edit update form is visible after clicking "Edit" button', async () => {
const {
getByText, getByRole, getAllByTestId, getAllByRole, queryByText,
getByText, getByRole, getAllByTestId, queryByText,
} = render(<RootWrapper />);
await waitFor(() => {
const editUpdateFirstButton = getAllByTestId('course-update-edit-button')[0];
const editButtons = getAllByRole('button', { name: 'Edit' });
const deleteButtons = getAllByRole('button', { name: 'Delete' });
const editUpdateButtons = getAllByTestId('course-update-edit-button');
const deleteButtons = getAllByTestId('course-update-delete-button');
const editHandoutsButtons = getAllByTestId('course-handouts-edit-button');
const editUpdateFirstButton = editUpdateButtons[0];
fireEvent.click(editUpdateFirstButton);
expect(getByText('Edit update')).toBeInTheDocument();
expect(getByRole('button', { name: messages.newUpdateButton.defaultMessage })).toBeDisabled();
editButtons.forEach((button) => expect(button).toBeDisabled());
editUpdateButtons.forEach((button) => expect(button).toBeDisabled());
editHandoutsButtons.forEach((button) => expect(button).toBeDisabled());
deleteButtons.forEach((button) => expect(button).toBeDisabled());
expect(queryByText(courseUpdatesMock[0].content)).not.toBeInTheDocument();
});

View File

@@ -1,6 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import { Button } from '@edx/paragon';
import { Icon, IconButtonWithTooltip } from '@edx/paragon';
import { EditOutline } from '@edx/paragon/icons';
import { useIntl } from '@edx/frontend-platform/i18n';
import messages from './messages';
@@ -12,16 +13,14 @@ const CourseHandouts = ({ contentForHandouts, onEdit, isDisabledButtons }) => {
<div className="course-handouts" data-testid="course-handouts">
<div className="course-handouts-header">
<h2 className="course-handouts-header__title lead">{intl.formatMessage(messages.handoutsTitle)}</h2>
<Button
className="course-handouts-header__btn"
data-testid="course-handouts-edit-button"
variant="outline-primary"
size="sm"
onClick={onEdit}
<IconButtonWithTooltip
tooltipContent={intl.formatMessage(messages.editButton)}
src={EditOutline}
iconAs={Icon}
disabled={isDisabledButtons}
>
{intl.formatMessage(messages.editButton)}
</Button>
data-testid="course-handouts-edit-button"
onClick={onEdit}
/>
</div>
<div
className="small"

View File

@@ -21,25 +21,25 @@ const renderComponent = (props) => render(
describe('<CourseHandouts />', () => {
it('render CourseHandouts component correctly', () => {
const { getByText, getByRole } = renderComponent();
const { getByText, getByTestId } = renderComponent();
expect(getByText(messages.handoutsTitle.defaultMessage)).toBeInTheDocument();
expect(getByText(handoutsContentMock)).toBeInTheDocument();
expect(getByRole('button', { name: messages.editButton.defaultMessage })).toBeInTheDocument();
expect(getByTestId('course-handouts-edit-button')).toBeInTheDocument();
});
it('calls the onEdit function when the edit button is clicked', () => {
const { getByRole } = renderComponent();
const { getByTestId } = renderComponent();
const editButton = getByRole('button', { name: messages.editButton.defaultMessage });
const editButton = getByTestId('course-handouts-edit-button');
fireEvent.click(editButton);
expect(onEditMock).toHaveBeenCalledTimes(1);
});
it('"Edit" button is disabled when isDisabledButtons is true', () => {
const { getByRole } = renderComponent({ isDisabledButtons: true });
const { getByTestId } = renderComponent({ isDisabledButtons: true });
const editButton = getByRole('button', { name: messages.editButton.defaultMessage });
const editButton = getByTestId('course-handouts-edit-button');
expect(editButton).toBeDisabled();
});
});

View File

@@ -1,8 +1,8 @@
import React from 'react';
import PropTypes from 'prop-types';
import { Button, Icon } from '@edx/paragon';
import { Icon, IconButtonWithTooltip } from '@edx/paragon';
import { useIntl } from '@edx/frontend-platform/i18n';
import { Error as ErrorIcon } from '@edx/paragon/icons/es5';
import { DeleteOutline, EditOutline, Error as ErrorIcon } from '@edx/paragon/icons';
import { isDateForUpdateValid } from './utils';
import messages from './messages';
@@ -27,18 +27,22 @@ const CourseUpdate = ({
</div>
)}
<div className="course-update-header__action">
<Button
variant="outline-primary"
size="sm"
onClick={onEdit}
<IconButtonWithTooltip
tooltipContent={intl.formatMessage(messages.editButton)}
src={EditOutline}
iconAs={Icon}
disabled={isDisabledButtons}
data-testid="course-update-edit-button"
>
{intl.formatMessage(messages.editButton)}
</Button>
<Button variant="outline-primary" size="sm" onClick={onDelete} disabled={isDisabledButtons}>
{intl.formatMessage(messages.deleteButton)}
</Button>
onClick={onEdit}
/>
<IconButtonWithTooltip
tooltipContent={intl.formatMessage(messages.deleteButton)}
src={DeleteOutline}
iconAs={Icon}
disabled={isDisabledButtons}
data-testid="course-update-delete-button"
onClick={onDelete}
/>
</div>
</div>
{Boolean(contentForUpdate) && (

View File

@@ -25,20 +25,20 @@ const renderComponent = (props) => render(
describe('<CourseUpdate />', () => {
it('render CourseUpdate component correctly', () => {
const { getByText, getByRole } = renderComponent();
const { getByText, getByTestId } = renderComponent();
expect(getByText(dateForUpdateMock)).toBeInTheDocument();
expect(getByRole('button', { name: messages.editButton.defaultMessage })).toBeInTheDocument();
expect(getByRole('button', { name: messages.deleteButton.defaultMessage })).toBeInTheDocument();
expect(getByTestId('course-update-edit-button')).toBeInTheDocument();
expect(getByTestId('course-update-delete-button')).toBeInTheDocument();
});
it('render CourseUpdate component without content correctly', () => {
const { getByText, queryByTestId, getByRole } = renderComponent({ contentForUpdate: '' });
const { getByText, queryByTestId, getByTestId } = renderComponent({ contentForUpdate: '' });
expect(getByText(dateForUpdateMock)).toBeInTheDocument();
expect(queryByTestId('course-update-content')).not.toBeInTheDocument();
expect(getByRole('button', { name: messages.editButton.defaultMessage })).toBeInTheDocument();
expect(getByRole('button', { name: messages.deleteButton.defaultMessage })).toBeInTheDocument();
expect(getByTestId('course-update-edit-button')).toBeInTheDocument();
expect(getByTestId('course-update-delete-button')).toBeInTheDocument();
});
it('render error message when dateForUpdate is inValid', () => {
@@ -48,25 +48,25 @@ describe('<CourseUpdate />', () => {
});
it('calls the onEdit function when the "Edit" button is clicked', () => {
const { getByRole } = renderComponent();
const { getByTestId } = renderComponent();
const editButton = getByRole('button', { name: messages.editButton.defaultMessage });
const editButton = getByTestId('course-update-edit-button');
fireEvent.click(editButton);
expect(onEditMock).toHaveBeenCalledTimes(1);
});
it('calls the onDelete function when the "Delete" button is clicked', () => {
const { getByRole } = renderComponent();
const { getByTestId } = renderComponent();
const deleteButton = getByRole('button', { name: messages.deleteButton.defaultMessage });
const deleteButton = getByTestId('course-update-delete-button');
fireEvent.click(deleteButton);
expect(onDeleteMock).toHaveBeenCalledTimes(1);
});
it('"Edit" and "Delete" buttons is disabled when isDisabledButtons is true', () => {
const { getByRole } = renderComponent({ isDisabledButtons: true });
const { getByTestId } = renderComponent({ isDisabledButtons: true });
expect(getByRole('button', { name: messages.editButton.defaultMessage })).toBeDisabled();
expect(getByRole('button', { name: messages.deleteButton.defaultMessage })).toBeDisabled();
expect(getByTestId('course-update-edit-button')).toBeDisabled();
expect(getByTestId('course-update-delete-button')).toBeDisabled();
});
});

View File

@@ -15,7 +15,6 @@ const DeleteModal = ({ isOpen, close, onDeleteSubmit }) => {
return (
<AlertModal
title={intl.formatMessage(messages.deleteModalTitle)}
variant="warning"
isOpen={isOpen}
onClose={close}
footerNode={(
@@ -29,7 +28,7 @@ const DeleteModal = ({ isOpen, close, onDeleteSubmit }) => {
onDeleteSubmit();
}}
>
{intl.formatMessage(messages.okButton)}
{intl.formatMessage(messages.deleteButton)}
</Button>
</ActionRow>
)}

View File

@@ -26,14 +26,14 @@ describe('<DeleteModal />', () => {
expect(getByText(messages.deleteModalTitle.defaultMessage)).toBeInTheDocument();
expect(getByText(messages.deleteModalDescription.defaultMessage)).toBeInTheDocument();
expect(getByRole('button', { name: messages.cancelButton.defaultMessage })).toBeInTheDocument();
expect(getByRole('button', { name: messages.okButton.defaultMessage })).toBeInTheDocument();
expect(getByRole('button', { name: messages.deleteButton.defaultMessage })).toBeInTheDocument();
});
it('calls onDeleteSubmit function when the "Ok" button is clicked', () => {
const { getByRole } = renderComponent();
const okButton = getByRole('button', { name: messages.okButton.defaultMessage });
fireEvent.click(okButton);
const deleteButton = getByRole('button', { name: messages.deleteButton.defaultMessage });
fireEvent.click(deleteButton);
expect(onDeleteSubmitMock).toHaveBeenCalledTimes(1);
});

View File

@@ -13,9 +13,9 @@ const messages = defineMessages({
id: 'course-authoring.course-updates.actions.cancel',
defaultMessage: 'Cancel',
},
okButton: {
id: 'course-authoring.course-updates.button.ok',
defaultMessage: 'Ok',
deleteButton: {
id: 'course-authoring.course-updates.button.delete',
defaultMessage: 'Delete',
},
});

View File

@@ -11,7 +11,7 @@ const messages = defineMessages({
},
sectionInfo: {
id: 'course-authoring.course-updates.section-info',
defaultMessage: 'Use course updates to notify students of important dates or exams, highlight particular discussions in the forums, announce schedule changes, and respond to student questions. You add or edit updates in HTML.',
defaultMessage: 'Use course updates to notify students of important dates or exams, highlight particular discussions in the forums, announce schedule changes, and respond to student questions.',
},
newUpdateButton: {
id: 'course-authoring.course-updates.actions.new-update',

View File

@@ -91,7 +91,7 @@ const UpdateForm = ({
</div>
{!isValid && (
<div className="datepicker-field-error">
<Icon src={ErrorIcon} alt={intl.formatMessage(messages.updateFormErrorAltText)} />
<Icon src={ErrorIcon} className="text-danger-500" alt={intl.formatMessage(messages.updateFormErrorAltText)} />
<span className="message-error">{intl.formatMessage(messages.updateFormInValid)}</span>
</div>
)}

View File

@@ -32,10 +32,6 @@
display: flex;
align-items: center;
gap: .25rem;
svg {
color: $warning-300;
}
}
.react-datepicker-popper {