From 617d26cb1854377475f16124b2fb5597bf1a5ecd Mon Sep 17 00:00:00 2001 From: KristinAoki Date: Tue, 19 Sep 2023 10:00:02 -0400 Subject: [PATCH 1/5] fix: change edit and delete buttons to icons --- src/course-updates/CourseUpdates.test.jsx | 36 ++++++++++--------- .../course-handouts/CourseHandouts.jsx | 17 +++++---- .../course-handouts/CourseHandouts.test.jsx | 12 +++---- .../course-update/CourseUpdate.jsx | 28 ++++++++------- .../course-update/CourseUpdate.test.jsx | 26 +++++++------- 5 files changed, 63 insertions(+), 56 deletions(-) diff --git a/src/course-updates/CourseUpdates.test.jsx b/src/course-updates/CourseUpdates.test.jsx index 46bdff9aa8..d136665e85 100644 --- a/src/course-updates/CourseUpdates.test.jsx +++ b/src/course-updates/CourseUpdates.test.jsx @@ -163,37 +163,39 @@ describe('', () => { }); it('Add new update form is visible after clicking "New update" button', async () => { - const { getByText, getByRole, getAllByRole } = render(); + const { getByText, getByRole, getAllByTestId } = render(); 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(); + const { getByText, getByRole, getAllByTestId } = render(); 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('', () => { it('Edit update form is visible after clicking "Edit" button', async () => { const { - getByText, getByRole, getAllByTestId, getAllByRole, queryByText, + getByText, getByRole, getAllByTestId, queryByText, } = render(); 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(); }); diff --git a/src/course-updates/course-handouts/CourseHandouts.jsx b/src/course-updates/course-handouts/CourseHandouts.jsx index ef374a01e3..2dc7d5ee16 100644 --- a/src/course-updates/course-handouts/CourseHandouts.jsx +++ b/src/course-updates/course-handouts/CourseHandouts.jsx @@ -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 }) => {

{intl.formatMessage(messages.handoutsTitle)}

- + />
render( describe('', () => { 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(); }); }); diff --git a/src/course-updates/course-update/CourseUpdate.jsx b/src/course-updates/course-update/CourseUpdate.jsx index dffc3d583e..51d8194e33 100644 --- a/src/course-updates/course-update/CourseUpdate.jsx +++ b/src/course-updates/course-update/CourseUpdate.jsx @@ -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 = ({
)}
- - + onClick={onEdit} + /> +
{Boolean(contentForUpdate) && ( diff --git a/src/course-updates/course-update/CourseUpdate.test.jsx b/src/course-updates/course-update/CourseUpdate.test.jsx index 32e4447296..9ce99248ba 100644 --- a/src/course-updates/course-update/CourseUpdate.test.jsx +++ b/src/course-updates/course-update/CourseUpdate.test.jsx @@ -25,20 +25,20 @@ const renderComponent = (props) => render( describe('', () => { 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('', () => { }); 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(); }); }); From 31dcc5e9ccf4a4b88dd3e3c9422254972fee5fad Mon Sep 17 00:00:00 2001 From: KristinAoki Date: Tue, 19 Sep 2023 10:04:09 -0400 Subject: [PATCH 2/5] fix: padding and button color --- src/course-updates/CourseUpdates.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/course-updates/CourseUpdates.jsx b/src/course-updates/CourseUpdates.jsx index 4167785260..ce63ade49e 100644 --- a/src/course-updates/CourseUpdates.jsx +++ b/src/course-updates/CourseUpdates.jsx @@ -63,7 +63,7 @@ const CourseUpdates = ({ courseId }) => { return ( <> - +
{ instruction={intl.formatMessage(messages.sectionInfo)} headerActions={( )} diff --git a/src/course-updates/delete-modal/DeleteModal.test.jsx b/src/course-updates/delete-modal/DeleteModal.test.jsx index bfd54e0bb0..54403cf603 100644 --- a/src/course-updates/delete-modal/DeleteModal.test.jsx +++ b/src/course-updates/delete-modal/DeleteModal.test.jsx @@ -26,14 +26,14 @@ describe('', () => { 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); }); diff --git a/src/course-updates/delete-modal/messages.js b/src/course-updates/delete-modal/messages.js index c8100d05be..af946884b3 100644 --- a/src/course-updates/delete-modal/messages.js +++ b/src/course-updates/delete-modal/messages.js @@ -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', }, }); From 90df9d85ef2dc2e5d3a45a42db94e5c938a1e28c Mon Sep 17 00:00:00 2001 From: KristinAoki Date: Tue, 19 Sep 2023 15:44:55 -0400 Subject: [PATCH 4/5] fix: date error icon color --- src/course-updates/update-form/UpdateForm.jsx | 2 +- src/course-updates/update-form/UpdateForm.scss | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/course-updates/update-form/UpdateForm.jsx b/src/course-updates/update-form/UpdateForm.jsx index dec0913e06..44eb42c527 100644 --- a/src/course-updates/update-form/UpdateForm.jsx +++ b/src/course-updates/update-form/UpdateForm.jsx @@ -91,7 +91,7 @@ const UpdateForm = ({ {!isValid && (
- + {intl.formatMessage(messages.updateFormInValid)}
)} diff --git a/src/course-updates/update-form/UpdateForm.scss b/src/course-updates/update-form/UpdateForm.scss index 82aed3feb7..a7a0ad0d15 100644 --- a/src/course-updates/update-form/UpdateForm.scss +++ b/src/course-updates/update-form/UpdateForm.scss @@ -32,10 +32,6 @@ display: flex; align-items: center; gap: .25rem; - - svg { - color: $warning-300; - } } .react-datepicker-popper { From 850cb7cd3097c91598f7731616c7210b87594785 Mon Sep 17 00:00:00 2001 From: KristinAoki Date: Fri, 22 Sep 2023 11:52:55 -0700 Subject: [PATCH 5/5] fix: page explanation text --- src/course-updates/messages.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/course-updates/messages.js b/src/course-updates/messages.js index b00a9d647c..a04a4b5cee 100644 --- a/src/course-updates/messages.js +++ b/src/course-updates/messages.js @@ -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',