Skip to content

Commit

Permalink
Use radio buttons on Task History instead of checkboxes (#4756)
Browse files Browse the repository at this point in the history
* Replace task history checkboxes with radio buttons + modify taskActivity tests

* Update default locale messages

* Adjust the vertical padding on task history components

Co-authored-by: Wille Marcel <wille.yyz@gmail.com>
  • Loading branch information
d-rita and willemarcel committed Jun 17, 2021
1 parent 5c88a0d commit acc6480
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 45 deletions.
10 changes: 7 additions & 3 deletions frontend/src/components/taskSelection/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,14 +332,18 @@ export default defineMessages({
id: 'project.tasks.action.history',
defaultMessage: 'History',
},
taskComments: {
taskHistoryComments: {
id: 'project.tasks.history.comments',
defaultMessage: 'Comments',
},
taskStateChanges: {
id: 'project.tasks.history.stateChanges',
taskHistoryActivities: {
id: 'project.tasks.history.activities',
defaultMessage: 'Activities',
},
taskHistoryAll: {
id: 'project.tasks.history.all',
defaultMessage: 'All',
},
copyComment: {
id: 'project.tasks.action.comments.copy',
defaultMessage: 'Copy comment',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const MultipleTaskHistoriesAccordion = ({ handleChange, tasks, projectId
<FormattedMessage {...messages.taskActivity} values={{ n: t.taskId }} />
</AccordionItemButton>
</AccordionItemHeading>
<AccordionItemPanel className="pa2 accordion_panel">
<AccordionItemPanel className="ph2 pb2 accordion_panel">
<TaskHistory projectId={projectId} taskId={t.taskId} commentPayload={undefined} />
</AccordionItemPanel>
</AccordionItem>
Expand Down
50 changes: 23 additions & 27 deletions frontend/src/components/taskSelection/taskActivity.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { ID_EDITOR_URL } from '../../config';
import { Button, CustomButton } from '../button';
import { Dropdown } from '../dropdown';
import { CommentInputField } from '../comments/commentInput';
import { CheckBoxInput } from '../formInputs';

const PostComment = ({ projectId, taskId, contributors, setCommentPayload }) => {
const token = useSelector((state) => state.auth.get('token'));
Expand Down Expand Up @@ -69,10 +68,9 @@ const PostComment = ({ projectId, taskId, contributors, setCommentPayload }) =>
export const TaskHistory = ({ projectId, taskId, commentPayload }) => {
const token = useSelector((state) => state.auth.get('token'));
const [history, setHistory] = useState([]);
const [showTaskComments, setShowTaskComments] = useState(true);
const [taskComments, setTaskComments] = useState([]);
const [showTaskChanges, setShowTaskChanges] = useState(false);
const [taskChanges, setTaskChanges] = useState([]);
const [historyOption, setHistoryOption] = useState('Comments');
const [shownHistory, setShownHistory] = useState([]);

useEffect(() => {
Expand Down Expand Up @@ -100,16 +98,14 @@ export const TaskHistory = ({ projectId, taskId, commentPayload }) => {
}, [projectId, taskId, token, commentPayload, getTaskInfo]);

useEffect(() => {
if (showTaskComments && showTaskChanges) {
setShownHistory(history);
} else if (showTaskComments) {
if (historyOption === 'Comments') {
setShownHistory(taskComments);
} else if (showTaskChanges) {
} else if (historyOption === 'Activities') {
setShownHistory(taskChanges);
} else {
setShownHistory(history);
}
}, [showTaskComments, showTaskChanges, taskComments, taskChanges, history]);
}, [historyOption, taskComments, taskChanges, history]);

const getTaskActionMessage = (action, actionText) => {
let message = '';
Expand Down Expand Up @@ -162,33 +158,33 @@ export const TaskHistory = ({ projectId, taskId, commentPayload }) => {
}
};

const taskHistoryOptions = [
{ value: 'Comments', label: 'Comments' },
{ value: 'Activities', label: 'Activities' },
{ value: 'All', label: 'All' },
];

if (!history) {
return null;
} else {
return (
<>
<div
className="ml3 pl1 pb3 blue-dark flex flex-wrap"
className="ml3 pl1 pv2 blue-dark flex flex-wrap"
aria-label="view task history options"
>
<div className="pt1 fl w-15" aria-labelledby="comments">
<CheckBoxInput
isActive={showTaskComments}
changeState={() => setShowTaskComments(!showTaskComments)}
/>
</div>
<span className="fl pt2 mr1 ph2" id="comments">
<FormattedMessage {...messages.taskComments} />
</span>
<div className="pt1 fl w-15" aria-labelledby="changes">
<CheckBoxInput
isActive={showTaskChanges}
changeState={() => setShowTaskChanges(!showTaskChanges)}
/>
</div>
<span className="fl pt2 mr1 ph2" id="changes">
<FormattedMessage {...messages.taskStateChanges} />
</span>
{taskHistoryOptions.map((option) => (
<label className="pt1 pr3 fl w-15" key={option.value}>
<input
value={option.value}
checked={historyOption === option.value}
onChange={() => setHistoryOption(option.value)}
type="radio"
className={`radio-input input-reset pointer v-mid dib h2 w2 mr2 br-100 ba b--blue-light`}
/>
<FormattedMessage {...messages[`taskHistory${option.label}`]} />
</label>
))}
</div>
{shownHistory.map((t, n) => (
<div className="w-90 mh3 pv3 bt b--grey-light f6 cf blue-dark" key={n}>
Expand Down
49 changes: 36 additions & 13 deletions frontend/src/components/taskSelection/tests/taskActivity.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,27 +48,38 @@ describe('TaskHistory', () => {
<TaskHistory projectId={2} taskId={15} commentPayload={history} />
</ReduxIntlProviders>,
);
expect(screen.getByText(/Activities/)).toBeInTheDocument();
expect(screen.getByText(/Comments/)).toBeInTheDocument();
expect(screen.getByText(/Activities/)).toBeInTheDocument();
expect(screen.getByText(/All/)).toBeInTheDocument();

// retrieve checkboxes
const historyCheckBoxes = screen.getAllByRole('checkbox');
expect(historyCheckBoxes[0]).toBeChecked(); // initial value of comments checkbox is checked
expect(historyCheckBoxes[1]).not.toBeChecked(); // initial value of activities checkbox is unchecked
expect(screen.getByText('User01')).toBeInTheDocument();
// retrieve radio buttons
const historyRadioButtons = screen.getAllByRole('radio'); //3 radio options: Comments, Activities and All

expect(historyRadioButtons[0]).toBeChecked(); // initial value of 'Comments' radio option is checked
expect(historyRadioButtons[1]).not.toBeChecked(); // initial value of 'Activities' radio option is unchecked
expect(historyRadioButtons[2]).not.toBeChecked(); // initial value of 'All' radio option is unchecked
expect(screen.getByText('commented 1 hour ago')).toBeInTheDocument();
expect(screen.getByText('missing buildings')).toBeInTheDocument();
expect(
screen.queryByText('marked as more mapping needed 1 minute ago'),
).not.toBeInTheDocument();
expect(screen.queryByText('locked for validation 2 hours ago')).not.toBeInTheDocument();

fireEvent.click(historyCheckBoxes[1]); // check activities checkbox
fireEvent.click(historyRadioButtons[1]); // check activities radio option
expect(historyRadioButtons[0]).not.toBeChecked();
expect(historyRadioButtons[2]).not.toBeChecked();
expect(screen.getByText('marked as more mapping needed 1 minute ago')).toBeInTheDocument();
expect(screen.getByText('locked for validation 2 hours ago')).toBeInTheDocument();
fireEvent.click(historyCheckBoxes[0]); // uncheck comments checkbox
expect(screen.queryByText('commented 1 hour ago')).not.toBeInTheDocument();
expect(screen.queryByText('missing buildings')).not.toBeInTheDocument();

fireEvent.click(historyRadioButtons[2]); // check All radio option
expect(historyRadioButtons[0]).not.toBeChecked();
expect(historyRadioButtons[1]).not.toBeChecked();
expect(screen.getByText('marked as more mapping needed 1 minute ago')).toBeInTheDocument();
expect(screen.getByText('locked for validation 2 hours ago')).toBeInTheDocument();
expect(screen.getByText('commented 1 hour ago')).toBeInTheDocument();
expect(screen.getByText('missing buildings')).toBeInTheDocument();
});

it('does not render any task history when not provided', () => {
Expand All @@ -82,11 +93,23 @@ describe('TaskHistory', () => {
<TaskHistory projectId={2} taskId={15} commentPayload={history} />
</ReduxIntlProviders>,
);
const historyCheckBoxes = screen.getAllByRole('checkbox');
fireEvent.click(historyCheckBoxes[1]); // check activities checkbox
expect(screen.getByText(/Activities/)).toBeInTheDocument();

expect(screen.getByText(/Comments/)).toBeInTheDocument();
expect(historyCheckBoxes[0]).toBeChecked();
expect(historyCheckBoxes[1]).toBeChecked();
expect(screen.getByText(/Activities/)).toBeInTheDocument();
expect(screen.getByText(/All/)).toBeInTheDocument();

const historyRadioButtons = screen.getAllByRole('radio'); //3 radio options: Comments, Activities and All

expect(historyRadioButtons[0]).toBeChecked(); // Comments radio option
expect(historyRadioButtons[1]).not.toBeChecked();
expect(historyRadioButtons[2]).not.toBeChecked();

fireEvent.click(historyRadioButtons[1]); // check activities radio option
expect(historyRadioButtons[0]).not.toBeChecked();
expect(historyRadioButtons[2]).not.toBeChecked();

fireEvent.click(historyRadioButtons[2]); // check All radio option
expect(historyRadioButtons[0]).not.toBeChecked();
expect(historyRadioButtons[1]).not.toBeChecked();
});
});
3 changes: 2 additions & 1 deletion frontend/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,8 @@
"project.tasks.action.completion": "Completion",
"project.tasks.action.history": "History",
"project.tasks.history.comments": "Comments",
"project.tasks.history.stateChanges": "Activities",
"project.tasks.history.activities": "Activities",
"project.tasks.history.all": "All",
"project.tasks.action.comments.copy": "Copy comment",
"project.tasks.action.comments.copy_to_all": "To all tasks",
"project.tasks.action.comments.copy_to_invalidated": "To tasks marked as \"No\"",
Expand Down

0 comments on commit acc6480

Please sign in to comment.