From b7c24d1e1ae31e5a463e99edd4c0ee3c484d55c2 Mon Sep 17 00:00:00 2001 From: Jesper Hodge <19345795+jesperhodge@users.noreply.github.com> Date: Fri, 13 Jan 2023 17:00:56 -0500 Subject: [PATCH] Fix answer box styling. TNL-10333 (#197) * fix: use feedback icon with correct hover color * fix: problem answer layout squishes delete button background * fix: remove borders from textarea * fix: textarea resize * refactor: remove renderThing-antipattern in answer option * fix: answer option feedback color * fix: add second feedback box to all problem types * refactor: move extra components out of answer option file * fix: icon disappearing on hover when active * fix: update snapshot * fix: lint * fix: add tests * fix: add tests * fix: snapshots * Update src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/AnswerOption.jsx Co-authored-by: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> * fix: resolve discussions from PR Co-authored-by: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> --- src/colors.scss | 2 + .../AnswerWidget/AnswerOption.jsx | 176 +++-------- .../__snapshots__/AnswerOption.test.jsx.snap | 283 +++++++----------- .../Checker/__snapshots__/index.test.jsx.snap | 23 ++ .../AnswerWidget/components/Checker/index.jsx | 32 ++ .../components/Checker/index.test.jsx | 22 ++ .../components/Feedback/FeedbackBox.jsx | 43 +++ .../components/Feedback/FeedbackBox.test.jsx | 22 ++ .../components/Feedback/FeedbackControl.jsx | 38 +++ .../Feedback/FeedbackControl.test.jsx | 26 ++ .../__snapshots__/FeedbackBox.test.jsx.snap | 66 ++++ .../FeedbackControl.test.jsx.snap | 33 ++ .../components/Feedback/index.jsx | 2 + .../components/Feedback/messages.js | 34 +++ .../EditProblemView/AnswerWidget/index.scss | 66 +++- src/index.scss | 2 +- 16 files changed, 549 insertions(+), 321 deletions(-) create mode 100644 src/colors.scss create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Checker/__snapshots__/index.test.jsx.snap create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Checker/index.jsx create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Checker/index.test.jsx create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackBox.jsx create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackBox.test.jsx create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackControl.jsx create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackControl.test.jsx create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/__snapshots__/FeedbackBox.test.jsx.snap create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/__snapshots__/FeedbackControl.test.jsx.snap create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/index.jsx create mode 100644 src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/messages.js diff --git a/src/colors.scss b/src/colors.scss new file mode 100644 index 000000000..a6630db18 --- /dev/null +++ b/src/colors.scss @@ -0,0 +1,2 @@ +$white: #fff; +$black: #000; \ No newline at end of file diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/AnswerOption.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/AnswerOption.jsx index 32e72ac69..725dbeddb 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/AnswerOption.jsx +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/AnswerOption.jsx @@ -2,57 +2,18 @@ import React, { memo } from 'react'; import { connect, useDispatch } from 'react-redux'; import PropTypes from 'prop-types'; import { - Col, Collapsible, Icon, IconButton, Form, Row, + Collapsible, Icon, IconButton, Form, } from '@edx/paragon'; -import { AddComment, Delete } from '@edx/paragon/icons'; -import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n'; +import { Feedback, Delete } from '@edx/paragon/icons'; +import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import messages from './messages'; import { selectors } from '../../../../../data/redux'; import { answerOptionProps } from '../../../../../data/services/cms/types'; -import { ProblemTypeKeys } from '../../../../../data/constants/problem'; +import Checker from './components/Checker'; +import { FeedbackBox } from './components/Feedback'; import * as hooks from './hooks'; -const Checker = ({ - hasSingleAnswer, answer, setAnswer, -}) => { - let CheckerType = Form.Checkbox; - if (hasSingleAnswer) { - CheckerType = Form.Radio; - } - return ( - setAnswer({ correct: e.target.checked })} - checked={answer.correct} - > - {answer.id} - - ); -}; - -const FeedbackControl = ({ - feedback, onChange, labelMessage, labelMessageBoldUnderline, key, answer, intl, -}) => ( - - - , - }} - /> - - - -); - export const AnswerOption = ({ answer, hasSingleAnswer, @@ -66,91 +27,56 @@ export const AnswerOption = ({ const setAnswer = hooks.setAnswer({ answer, hasSingleAnswer, dispatch }); const { isFeedbackVisible, toggleFeedback } = hooks.prepareFeedback(answer); - const displayFeedbackControl = (answerObject) => { - if (problemType !== ProblemTypeKeys.MULTISELECT) { - return FeedbackControl({ - key: `feedback-${answerObject.id}`, - feedback: answerObject.selectedFeedback, - onChange: (e) => setAnswer({ selectedFeedback: e.target.value }), - labelMessage: messages.selectedFeedbackLabel, - labelMessageBoldUnderline: messages.selectedFeedbackLabelBoldUnderlineText, - answer: answerObject, - intl, - }); - } - return [ - FeedbackControl({ - key: `selectedfeedback-${answerObject.id}`, - feedback: answerObject.selectedFeedback, - onChange: (e) => setAnswer({ selectedFeedback: e.target.value }), - labelMessage: messages.selectedFeedbackLabel, - labelMessageBoldUnderline: messages.selectedFeedbackLabelBoldUnderlineText, - answer: answerObject, - intl, - }), - FeedbackControl({ - key: `unselectedfeedback-${answerObject.id}`, - feedback: answerObject.unselectedFeedback, - onChange: (e) => setAnswer({ unselectedFeedback: e.target.value }), - labelMessage: messages.unSelectedFeedbackLabel, - labelMessageBoldUnderline: messages.unSelectedFeedbackLabelBoldUnderlineText, - answer: answerObject, - intl, - }), - ]; - }; - return ( - - - - + + +
+ { setAnswer({ title: e.target.value }); }} + placeholder={intl.formatMessage(messages.answerTextboxPlaceholder)} + /> + + - - - - { setAnswer({ title: e.target.value }); }} - placeholder={intl.formatMessage(messages.answerTextboxPlaceholder)} - /> - - -
- {displayFeedbackControl(answer)} -
-
- - - - - - +
+
+
+ - - - + + +
); }; @@ -164,22 +90,6 @@ AnswerOption.propTypes = { problemType: PropTypes.string.isRequired, }; -FeedbackControl.propTypes = { - feedback: PropTypes.string.isRequired, - onChange: PropTypes.func.isRequired, - labelMessage: PropTypes.string.isRequired, - labelMessageBoldUnderline: PropTypes.string.isRequired, - key: PropTypes.string.isRequired, - answer: answerOptionProps.isRequired, - intl: intlShape.isRequired, -}; - -Checker.propTypes = { - hasSingleAnswer: PropTypes.bool.isRequired, - answer: answerOptionProps.isRequired, - setAnswer: PropTypes.func.isRequired, -}; - export const mapStateToProps = (state) => ({ problemType: selectors.problem.problemType(state), }); diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/__snapshots__/AnswerOption.test.jsx.snap b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/__snapshots__/AnswerOption.test.jsx.snap index 4396f8e4b..c6bf42cc8 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/__snapshots__/AnswerOption.test.jsx.snap +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/__snapshots__/AnswerOption.test.jsx.snap @@ -2,17 +2,40 @@ exports[`AnswerOption render snapshot: renders correct option with feedback 1`] = ` - - - + +
+ + + - - - - -
- - - - - - - , - } - } - /> - - - -
- - - - - - + +
+
+ - - + + +
`; exports[`AnswerOption render snapshot: renders correct option with selected unselected feedback 1`] = ` - - - + +
+ + + - - - - -
- - - - - - - , - } - } - /> - - - - - - - - - - , - } - } - /> - - - -
- - - - - - + +
+
+ - - + + +
`; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Checker/__snapshots__/index.test.jsx.snap b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Checker/__snapshots__/index.test.jsx.snap new file mode 100644 index 000000000..d8108e18d --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Checker/__snapshots__/index.test.jsx.snap @@ -0,0 +1,23 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Checker component with multiple answers 1`] = ` + + A + +`; + +exports[`Checker component with single answer 1`] = ` + + A + +`; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Checker/index.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Checker/index.jsx new file mode 100644 index 000000000..de39a3d87 --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Checker/index.jsx @@ -0,0 +1,32 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { Form } from '@edx/paragon'; + +const Checker = ({ + hasSingleAnswer, answer, setAnswer, +}) => { + let CheckerType = Form.Checkbox; + if (hasSingleAnswer) { + CheckerType = Form.Radio; + } + return ( + setAnswer({ correct: e.target.checked })} + checked={answer.correct} + > + {answer.id} + + ); +}; +Checker.propTypes = { + hasSingleAnswer: PropTypes.bool.isRequired, + answer: PropTypes.shape({ + correct: PropTypes.bool, + id: PropTypes.number, + }).isRequired, + setAnswer: PropTypes.func.isRequired, +}; + +export default Checker; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Checker/index.test.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Checker/index.test.jsx new file mode 100644 index 000000000..19998a536 --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Checker/index.test.jsx @@ -0,0 +1,22 @@ +import { shallow } from 'enzyme'; +import Checker from '.'; + +const props = { + hasSingleAnswer: true, + answer: { + id: 'A', + title: 'Answer 1', + correct: true, + selectedFeedback: 'some feedback', + }, + setAnswer: jest.fn(), +}; +describe('Checker component', () => { + test('with single answer', () => { + expect(shallow()).toMatchSnapshot(); + }); + + test('with multiple answers', () => { + expect(shallow()).toMatchSnapshot(); + }); +}); diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackBox.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackBox.jsx new file mode 100644 index 000000000..4101f944a --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackBox.jsx @@ -0,0 +1,43 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; + +import { answerOptionProps } from '../../../../../../../data/services/cms/types'; +import FeedbackControl from './FeedbackControl'; +import { messages } from './messages'; + +export const FeedbackBox = ({ + answer, setAnswer, intl, +}) => { + const props = { + onChange: (e) => setAnswer({ selectedFeedback: e.target.value }), + answer, + intl, + }; + + return ( +
+ + +
+ ); +}; +FeedbackBox.propTypes = { + answer: answerOptionProps.isRequired, + setAnswer: PropTypes.func.isRequired, + intl: intlShape.isRequired, +}; + +export default injectIntl(FeedbackBox); diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackBox.test.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackBox.test.jsx new file mode 100644 index 000000000..117c48255 --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackBox.test.jsx @@ -0,0 +1,22 @@ +import { shallow } from 'enzyme'; +import { FeedbackBox } from './FeedbackBox'; + +const answerWithFeedback = { + id: 'A', + title: 'Answer 1', + correct: true, + selectedFeedback: 'some feedback', + unselectedFeedback: 'unselectedFeedback', +}; + +const props = { + answer: answerWithFeedback, + intl: {}, + setAnswer: jest.fn(), +}; + +describe('FeedbackBox component', () => { + test('renders', () => { + expect(shallow()).toMatchSnapshot(); + }); +}); diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackControl.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackControl.jsx new file mode 100644 index 000000000..12ff8091d --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackControl.jsx @@ -0,0 +1,38 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { FormattedMessage, intlShape } from '@edx/frontend-platform/i18n'; +import { Form } from '@edx/paragon'; + +import { answerOptionProps } from '../../../../../../../data/services/cms/types'; +import messages from './messages'; + +const FeedbackControl = ({ + feedback, onChange, labelMessage, labelMessageBoldUnderline, answer, intl, +}) => ( + + + , + }} + /> + + + +); +FeedbackControl.propTypes = { + feedback: PropTypes.string.isRequired, + onChange: PropTypes.func.isRequired, + labelMessage: PropTypes.string.isRequired, + labelMessageBoldUnderline: PropTypes.string.isRequired, + answer: answerOptionProps.isRequired, + intl: intlShape.isRequired, +}; + +export default FeedbackControl; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackControl.test.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackControl.test.jsx new file mode 100644 index 000000000..93d6499d4 --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/FeedbackControl.test.jsx @@ -0,0 +1,26 @@ +import { shallow } from 'enzyme'; +import FeedbackControl from './FeedbackControl'; + +const answerWithFeedback = { + id: 'A', + title: 'Answer 1', + correct: true, + selectedFeedback: 'some feedback', + unselectedFeedback: 'unselectedFeedback', +}; + +const props = { + answer: answerWithFeedback, + intl: { formatMessage: jest.fn() }, + setAnswer: jest.fn(), + feedback: 'feedback', + onChange: jest.fn(), + labelMessage: 'msg', + labelMessageBoldUnderline: 'msg', +}; + +describe('FeedbackControl component', () => { + test('renders', () => { + expect(shallow()).toMatchSnapshot(); + }); +}); diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/__snapshots__/FeedbackBox.test.jsx.snap b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/__snapshots__/FeedbackBox.test.jsx.snap new file mode 100644 index 000000000..ae9ffb472 --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/__snapshots__/FeedbackBox.test.jsx.snap @@ -0,0 +1,66 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`FeedbackBox component renders 1`] = ` +
+ + +
+`; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/__snapshots__/FeedbackControl.test.jsx.snap b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/__snapshots__/FeedbackControl.test.jsx.snap new file mode 100644 index 000000000..40a7fde43 --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/__snapshots__/FeedbackControl.test.jsx.snap @@ -0,0 +1,33 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`FeedbackControl component renders 1`] = ` + + + + + + + , + } + } + /> + + + +`; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/index.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/index.jsx new file mode 100644 index 000000000..bce02835a --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/index.jsx @@ -0,0 +1,2 @@ +export { default as FeedbackBox } from './FeedbackBox'; +export { default as FeedbackControl } from './FeedbackControl'; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/messages.js b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/messages.js new file mode 100644 index 000000000..a580c9c7e --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/components/Feedback/messages.js @@ -0,0 +1,34 @@ +export const messages = { + feedbackPlaceholder: { + id: 'authoring.answerwidget.feedback.placeholder', + defaultMessage: 'Feedback message', + description: 'Placeholder text for feedback text', + }, + feedbackToggleIconAltText: { + id: 'authoring.answerwidget.feedback.icon.alt', + defaultMessage: 'Toggle feedback', + description: 'Alt text for feedback toggle icon', + }, + selectedFeedbackLabel: { + id: 'authoring.answerwidget.feedback.selected.label', + defaultMessage: 'Show following feedback when {answerId} {boldunderline}:', + description: 'Label text for feedback if option is selected', + }, + selectedFeedbackLabelBoldUnderlineText: { + id: 'authoring.answerwidget.feedback.selected.label.boldunderline', + defaultMessage: 'is selected', + description: 'Bold & underlined text for feedback if option is selected', + }, + unSelectedFeedbackLabel: { + id: 'authoring.answerwidget.feedback.unselected.label', + defaultMessage: 'Show following feedback when {answerId} {boldunderline}:', + description: 'Label text for feedback if option is not selected', + }, + unSelectedFeedbackLabelBoldUnderlineText: { + id: 'authoring.answerwidget.feedback.unselected.label.boldunderline', + defaultMessage: 'is not selected', + description: 'Bold & underlined text for feedback if option is not selected', + }, +}; + +export default messages; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/index.scss b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/index.scss index 622a591a9..bd272ed1e 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/index.scss +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/index.scss @@ -1,11 +1,59 @@ -.problem-answer { - padding: 12px; - color: #00262B; - .problem-answer-title { - font-weight: bold; - } +@import '../../../../../../colors.scss'; - .problem-answer-description { - font-size: 0.9rem; - } +.problem-answer { + padding: 12px; + + .problem-answer-title { + font-weight: bold; + } + + .problem-answer-description { + font-size: 0.9rem; + } } + +.answer-option { + // The paragon icon path is missing "fill: currentColor" + .feedback-icon-button { + &:hover { + &:not(:focus) { + svg { + path { + fill: $white; + } + } + } + } + } + + .answer-option-flex-item-1 { + flex-basis: 8.33%; + } + + .answer-option-flex-item-2 { + flex-basis: 83.34%; + } + + .answer-option-flex-item-3 { + flex-basis: 8.33%; + } + + .answer-option-textarea { + textarea { + border: none; + resize: none; + + &:active, &:hover, &:focus, &:focus-visible { + border: none; + border-right: none; + border-left: none; + border-radius: 0; + box-shadow: none; + } + + &:focus, &:hover { + border-bottom: 1px solid $black; + } + } + } +} \ No newline at end of file diff --git a/src/index.scss b/src/index.scss index c4301db28..491817e5c 100644 --- a/src/index.scss +++ b/src/index.scss @@ -1 +1 @@ -/* styles go here */ +@import './colors.scss'; \ No newline at end of file