Skip to content

Commit

Permalink
feat: add loading circles to modals (#21)
Browse files Browse the repository at this point in the history
* feat: add loading circles to modals

- reset, reject, and verify buttons all have loading circles now
- also, renamed the components to modals since that's what they are

* chore: lint

* test: update snapshots

* test: remove old snapshots

* feat: button labels moved to translation msgs

* temp: trying to link button state to request

* temp: 2nd attempt of hooks in component

* docs: comments for request initial states

* feat: cleaned code, moved redux hook -> utils

* temp: rollback point for moving utils code

* temp: tried moving utils to redux folder

- it didn't work

* chore: nit

* temp: moving req. status to custom hook

* fix: mocks in tests for new hook

* temp: trying to make new test work

* temp: fiddling with test

* test: complete test for button state

* test: hooks test

* test: added reset tests

- also fixed some nits

* chore: lint

* docs: added button state description

* chore: removed unecessary mock

* chore: nit
  • Loading branch information
ilee2u authored Jan 10, 2024
1 parent 9ce2fce commit 2ea6ad1
Show file tree
Hide file tree
Showing 16 changed files with 285 additions and 116 deletions.
10 changes: 10 additions & 0 deletions src/data/redux/requests/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,18 @@ import { createSlice } from '@reduxjs/toolkit';

import { RequestStates, RequestKeys } from 'data/constants';

// NOTE: Anytime a new request is coded, or when a request has its RequestKey changed,
// It must be added to this variable in the following format:
// [RequestKeys.fetchCourseExams]: { status: RequestStates.inactive }
// This order to prevents an error form being thrown when we call for the status of these requests,
// as calling for the status of an undefined/yet-to-be called request will throw an error.
// See src/data/redux/requests/selectors.js for more info.
const initialState = {
[RequestKeys.fetchCourseExams]: { status: RequestStates.inactive },
[RequestKeys.fetchExamAttempts]: { status: RequestStates.inactive },
[RequestKeys.deleteExamAttempt]: { status: RequestStates.inactive },
[RequestKeys.modifyExamAttempt]: { status: RequestStates.inactive },
[undefined]: { status: RequestStates.inactive },
};

// eslint-disable-next-line no-unused-vars
Expand Down
10 changes: 9 additions & 1 deletion src/data/redux/requests/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,15 @@ import { RequestStates } from 'data/constants';

export const requestStatus = (state, { requestKey }) => state.requests[requestKey];

const statusSelector = (fn) => (requestKey) => (state) => fn(state.requests[requestKey]);
const statusSelector = (fn) => (requestKey) => (state) => {
if (state.requests[requestKey] === undefined) {
throw new TypeError(`The request with RequestKey ${requestKey} is missing an initial state. \
If you see this error, then you must initialize your request statuses in the initialState variable in \
src/data/redux/requests/reducer.js \n\n \
This error replaces this otherwise vague error: TypeError: Cannot destructure property 'status' of '_ref3' as it is undefined.`);
}
return fn(state.requests[requestKey]);
};

export const isInactive = ({ status }) => status === RequestStates.inactive;
export const isPending = ({ status }) => status === RequestStates.pending;
Expand Down
18 changes: 9 additions & 9 deletions src/pages/ExamsPage/components/AttemptList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import PropTypes from 'prop-types';
import { DataTable, TextFilter, CheckboxFilter } from '@edx/paragon';
import { useIntl } from '@edx/frontend-platform/i18n';
import * as constants from 'data/constants';
import ResetExamAttemptButton from './ResetExamAttemptButton';
import ReviewExamAttemptButton from './ReviewExamAttemptButton';
import ResetExamAttemptModal from './ResetExamAttemptModal';
import ReviewExamAttemptModal from './ReviewExamAttemptModal';
import messages from '../messages';

// TODO: these should be updated to use intl messages
Expand Down Expand Up @@ -81,17 +81,17 @@ const StatusFilterChoices = [
},
];

// The button components must be compartmentalized here otherwise npm lint throws an unstable-nested-component error.
const ResetButton = (row) => (
<ResetExamAttemptButton
// The modal components must be compartmentalized here otherwise npm lint throws an unstable-nested-component error.
const ResetModal = (row) => (
<ResetExamAttemptModal
username={row.original.username}
examName={row.original.exam_name}
attemptId={row.original.attempt_id}
/>
);

const ReviewButton = (row) => (
<ReviewExamAttemptButton
const ReviewModal = (row) => (
<ReviewExamAttemptModal
username={row.original.username}
examName={row.original.exam_name}
attemptId={row.original.attempt_id}
Expand Down Expand Up @@ -120,12 +120,12 @@ const AttemptList = ({ attempts }) => {
{
id: 'action',
Header: formatMessage(messages.examAttemptsTableHeaderAction),
Cell: ({ row }) => ResetButton(row),
Cell: ({ row }) => ResetModal(row),
},
{
id: 'review',
Header: formatMessage(messages.examAttemptsTableHeaderReview),
Cell: ({ row }) => ReviewButton(row),
Cell: ({ row }) => ReviewModal(row),
},
]}
data={attempts}
Expand Down
2 changes: 2 additions & 0 deletions src/pages/ExamsPage/components/AttemptList.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ jest.mock('../hooks', () => ({
useDeleteExamAttempt: jest.fn(),
useModifyExamAttempt: jest.fn(),
useExamsData: jest.fn(),
useButtonStateFromRequestStatus: jest.fn(),
}));

describe('AttemptList', () => {
beforeEach(() => {
hooks.useDeleteExamAttempt.mockReturnValue(jest.fn());
hooks.useModifyExamAttempt.mockReturnValue(jest.fn());
hooks.useButtonStateFromRequestStatus.mockReturnValue(jest.fn());
hooks.useExamsData.mockReturnValue(testUtils.defaultExamsData);
});
it('Test that the AttemptList matches snapshot', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,34 @@
import PropTypes from 'prop-types';

import {
Button, useToggle, ModalDialog, ActionRow,
Button, useToggle, ModalDialog, ActionRow, StatefulButton,
} from '@edx/paragon';
import { useIntl } from '@edx/frontend-platform/i18n';
import { useDeleteExamAttempt } from '../hooks';
import * as constants from 'data/constants';
import { useDeleteExamAttempt, useButtonStateFromRequestStatus } from '../hooks';
import messages from '../messages';

const ResetExamAttemptButton = ({ username, examName, attemptId }) => {
const ResetExamAttemptModal = ({ username, examName, attemptId }) => {
const [isOpen, open, close] = useToggle(false);
const resetExamAttempt = useDeleteExamAttempt();
const getRequestStatus = useButtonStateFromRequestStatus();
const { formatMessage } = useIntl();

const ResetButtonProps = {
labels: {
default: formatMessage(messages.ResetExamAttemptButtonDefaultLabel),
pending: formatMessage(messages.ResetExamAttemptButtonPendingLabel),
complete: formatMessage(messages.ResetExamAttemptButtonCompelteLabel),
error: formatMessage(messages.ResetExamAttemptButtonErrorLabel),
},
variant: 'primary',
};

return (
<>
<div className="d-flex">
<Button variant="link" size="sm" onClick={open}>
{formatMessage(messages.ResetExamAttemptButtonTitle)}
{formatMessage(messages.ResetExamAttemptModalTitle)}
</Button>
</div>
<ModalDialog
Expand All @@ -30,12 +42,12 @@ const ResetExamAttemptButton = ({ username, examName, attemptId }) => {
>
<ModalDialog.Header>
<ModalDialog.Title>
{formatMessage(messages.ResetExamAttemptButtonModalTitle)}
{formatMessage(messages.ResetExamAttemptModalModalTitle)}
</ModalDialog.Title>
</ModalDialog.Header>

<ModalDialog.Body>
<p>{formatMessage(messages.ResetExamAttemptButtonModalBody)}</p>
<p>{formatMessage(messages.ResetExamAttemptModalModalBody)}</p>
<ul>
<li>{formatMessage(messages.Username)}{username}</li>
<li>{formatMessage(messages.ExamName)}{examName}</li>
Expand All @@ -45,27 +57,30 @@ const ResetExamAttemptButton = ({ username, examName, attemptId }) => {
<ModalDialog.Footer>
<ActionRow>
<ModalDialog.CloseButton variant="tertiary">
{formatMessage(messages.ResetExamAttemptButtonCancel)}
{formatMessage(messages.ResetExamAttemptModalCancel)}
</ModalDialog.CloseButton>
<Button
<StatefulButton
data-testid="reset-stateful-button"
// The state of this button is updated based on the request status of the deleteExamAttempt
// api function. The change of the button's label is based on VerifyButtonProps
state={getRequestStatus(constants.RequestKeys.deleteExamAttempt)}
{...ResetButtonProps}
variant="primary"
onClick={e => { // eslint-disable-line no-unused-vars
resetExamAttempt(attemptId);
}}
>
{formatMessage(messages.ResetExamAttemptButtonConfirm)}
</Button>
/>
</ActionRow>
</ModalDialog.Footer>
</ModalDialog>
</>
);
};

ResetExamAttemptButton.propTypes = {
ResetExamAttemptModal.propTypes = {
username: PropTypes.string.isRequired,
examName: PropTypes.string.isRequired,
attemptId: PropTypes.number.isRequired,
};

export default ResetExamAttemptButton;
export default ResetExamAttemptModal;
Original file line number Diff line number Diff line change
@@ -1,46 +1,56 @@
import { render, screen } from '@testing-library/react';

import ResetExamAttemptButton from './ResetExamAttemptButton';
import ResetExamAttemptModal from './ResetExamAttemptModal';

import * as hooks from '../hooks';

jest.mock('../hooks', () => ({
useDeleteExamAttempt: jest.fn(),
useButtonStateFromRequestStatus: jest.fn(),
}));

const mockMakeNetworkRequest = jest.fn();

// nomally mocked for unit tests but required for rendering/snapshots
jest.unmock('react');

const resetButton = <ResetExamAttemptButton username="username" examName="examName" attemptId={0} />;
const resetModal = <ResetExamAttemptModal username="username" examName="examName" attemptId={0} />;

describe('ResetExamAttemptButton', () => {
describe('ResetExamAttemptModal', () => {
beforeEach(() => {
jest.restoreAllMocks();
hooks.useDeleteExamAttempt.mockReturnValue(mockMakeNetworkRequest);
hooks.useButtonStateFromRequestStatus.mockReturnValue(mockMakeNetworkRequest);
});
it('Test that the ResetExamAttemptButton matches snapshot', () => {
expect(render(resetButton)).toMatchSnapshot();
it('Test that the ResetExamAttemptModal matches snapshot', () => {
expect(render(resetModal)).toMatchSnapshot();
});
it('Modal appears upon clicking button', () => {
render(resetButton);
render(resetModal);
screen.getByText('Reset').click();
expect(screen.getByText('Please confirm your choice.')).toBeInTheDocument();
});
it('Clicking the No button closes the modal', () => {
render(resetButton);
render(resetModal);
screen.getByText('Reset').click();
screen.getByText('Cancel').click();
// Using queryByText here allows the function to throw
expect(screen.queryByText('Please confirm your choice.')).not.toBeInTheDocument();
});
it('Clicking the Reset button displays the correct label based on the request state', () => {
render(resetModal);
hooks.useButtonStateFromRequestStatus.mockReturnValue(() => 'pending'); // for testing button label state
screen.getByText('Reset').click();
expect(screen.queryByText('Resetting...')).toBeInTheDocument(); // The button should be in the pending state
});
it('Clicking the Yes button calls the deletion hook', () => {
const mockDeleteExamAttempt = jest.fn();
jest.spyOn(hooks, 'useDeleteExamAttempt').mockImplementation(() => mockDeleteExamAttempt);
render(resetButton);
render(resetModal);
screen.getByText('Reset').click();
screen.getByText('Yes, I\'m Sure').click();
// Need to use a test id here b/c there are two button with label 'Reset',
// One in the table, and one in the modal
screen.getByTestId('reset-stateful-button').click();
expect(mockDeleteExamAttempt).toHaveBeenCalledWith(0);
});
});
Loading

0 comments on commit 2ea6ad1

Please sign in to comment.