From 24215fd10b216e3d007cbb38d88c80e73f00a2c3 Mon Sep 17 00:00:00 2001 From: efloden Date: Fri, 16 Feb 2018 16:51:55 +1000 Subject: [PATCH 1/7] feat(ZNTA-2392): enforce comment required criteria option disable reject translation button until all requirements met add disabled style to the reject translation button --- .../CriteriaDropdown.js | 22 +++++++-- .../CriteriaDropdown.test.js | 15 +++++- .../RejectTranslationModal/index.css | 6 +++ .../RejectTranslationModal/index.js | 46 ++++++++++++++----- 4 files changed, 72 insertions(+), 17 deletions(-) diff --git a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/CriteriaDropdown.js b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/CriteriaDropdown.js index d26bb9b8ea..365b1639d9 100644 --- a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/CriteriaDropdown.js +++ b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/CriteriaDropdown.js @@ -4,6 +4,7 @@ import { Icon } from '../../../components' import * as PropTypes from 'prop-types' import Dropdown from '../../components/Dropdown' import { MINOR, MAJOR, CRITICAL } from '../../utils/reject-trans-util' +import { UNSPECIFIED } from './index' /** * A Local Editor Dropdown coponent that selects the Criteria @@ -17,7 +18,8 @@ class CriteriaDropdown extends Component { priority: PropTypes.oneOf([MINOR, MAJOR, CRITICAL]).isRequired })).isRequired, onCriteriaChange: PropTypes.func.isRequired, - selectedCriteria: PropTypes.string.isRequired + onUnspecifiedCriteria: PropTypes.func.isRequired, + criteriaDescription: PropTypes.string.isRequired } constructor (props) { super(props) @@ -34,24 +36,36 @@ class CriteriaDropdown extends Component { this.props.onCriteriaChange(event) this.toggleDropdown() } + onUnspecifiedCriteria = () => { + this.props.onUnspecifiedCriteria() + this.toggleDropdown() + } render () { - const { criteriaList, selectedCriteria } = this.props + const { criteriaList, criteriaDescription } = this.props const options = criteriaList.map((value, index) => { return ( -
  • {value.description}
    • ) }) + // FIXME: should not be modifying the options array + options.unshift( +
  • + {UNSPECIFIED} +
    • + ) return ( - {selectedCriteria} + {criteriaDescription} diff --git a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/CriteriaDropdown.test.js b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/CriteriaDropdown.test.js index eeaebf7875..1df28b4911 100644 --- a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/CriteriaDropdown.test.js +++ b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/CriteriaDropdown.test.js @@ -4,6 +4,7 @@ import CriteriaDropdown from './CriteriaDropdown' import { MINOR, MAJOR, CRITICAL } from '../../utils/reject-trans-util' import { Icon } from '../../../components' import Dropdown from '../../components/Dropdown' +import { UNSPECIFIED } from './index' const defaultClick = () => {} const criteriaList = [{ @@ -21,13 +22,22 @@ const criteriaList = [{ }] const options = criteriaList.map((value, index) => { return ( -
  • {value.description}
    • ) }) + +// FIXME: should not be modifying the options array +options.unshift( +
  • + {UNSPECIFIED} +
    • +) /* global describe it expect */ describe('CriteriaDropdown', () => { it('renders default markup', () => { @@ -35,7 +45,8 @@ describe('CriteriaDropdown', () => { ) const expected = ReactDOMServer.renderToStaticMarkup( diff --git a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/index.css b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/index.css index fcb09d7919..aa7fec9923 100644 --- a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/index.css +++ b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/index.css @@ -7,6 +7,7 @@ --TM-text-color: rgb(84, 102, 119); --TM-icon-vert-alignment: middle; --TM-dropdown-text-color: #03a6d7; + --Button-color-neutral: color(#416988 tint(50%)); } #RejectTranslationModal .flex { @@ -85,6 +86,11 @@ visibility: hidden; } +#RejectTranslationModal .EditorButton:disabled { + color: var(--Button-color-text-invert); + background-color: var(--Button-color-neutral); +} + .EditorRejection-input { margin-top: 1rem; padding: 0.75rem; diff --git a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/index.js b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/index.js index 7038fb0030..725275c794 100644 --- a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/index.js +++ b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/index.js @@ -15,6 +15,7 @@ import { MINOR, MAJOR, CRITICAL, priorities, textState } from '../../utils/reject-trans-util' +export const UNSPECIFIED = 'Unspecified Criteria' const textLimit = 500 /** @@ -41,11 +42,17 @@ export class RejectTranslationModal extends Component { review: { selectedPriority: MINOR, priorityId: 0, - selectedCriteria: '-- Select a predefined criteria --', + criteriaDescription: '-- Select a predefined criteria --', criteriaId: undefined, reviewComment: '' }, - charsLeft: textLimit + charsLeft: textLimit, + selectedCriteria: { + id: undefined, + editable: false, + description: '', + priority: MINOR + } } constructor (props) { super(props) @@ -62,14 +69,26 @@ export class RejectTranslationModal extends Component { })) } onCriteriaChange = (event) => { - const selectedCriteria = event.target.innerText - const criteriaId = this.props.criteriaList.find( - x => x.description === event.target.innerText).id + const selectedCriteria = this.props.criteriaList.find( + x => x.description === event.target.innerText) this.setState(prevState => ({ review: update(prevState.review, { - selectedCriteria: {$set: selectedCriteria}, - criteriaId: {$set: criteriaId} - }) + criteriaDescription: {$set: selectedCriteria.description}, + criteriaId: {$set: selectedCriteria.id}, + selectedPriority: {$set: selectedCriteria.priority} + }), + selectedCriteria: selectedCriteria + })) + } + onUnspecifiedCriteria = () => { + const unspecifiedCriteria = this.defaultState.selectedCriteria + this.setState(prevState => ({ + review: update(prevState.review, { + criteriaDescription: {$set: UNSPECIFIED}, + criteriaId: {$set: undefined}, + selectedPriority: {$set: unspecifiedCriteria.priority} + }), + selectedCriteria: unspecifiedCriteria })) } setReviewComment = (event) => { @@ -106,7 +125,7 @@ export class RejectTranslationModal extends Component { /* eslint-disable max-len */ render () { const { show, criteriaList } = this.props - const { review } = this.state + const { review, selectedCriteria } = this.state const priorityTextState = textState(review.selectedPriority) const criteriaTile = (!isEmpty(criteriaList)) ?
    @@ -116,13 +135,17 @@ export class RejectTranslationModal extends Component { + onUnspecifiedCriteria={this.onUnspecifiedCriteria} + criteriaDescription={review.criteriaDescription} />
    : undefined + const canReject = ( + (!isEmpty(review.reviewComment)) || + (selectedCriteria.editable === false)) return ( From 5ccba8e8040f31edf45105d3423b761c6e14de4e Mon Sep 17 00:00:00 2001 From: efloden Date: Mon, 19 Feb 2018 10:44:49 +1000 Subject: [PATCH 2/7] refactor(ZNTA-2392): initialize unspecified criteria option in reducer change placeholder text when user must input a comment --- .../CriteriaDropdown.js | 25 ++++----- .../CriteriaDropdown.test.js | 12 +---- .../RejectTranslationModal.story.js | 51 ------------------- .../RejectTranslationModal/index.js | 33 +++++------- .../editor/reducers/review-trans-reducer.js | 3 ++ .../reducers/review-trans-reducer.test.js | 10 +++- .../src/app/editor/utils/reject-trans-util.js | 8 +++ 7 files changed, 43 insertions(+), 99 deletions(-) delete mode 100644 server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/RejectTranslationModal.story.js diff --git a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/CriteriaDropdown.js b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/CriteriaDropdown.js index 365b1639d9..03042cb5a8 100644 --- a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/CriteriaDropdown.js +++ b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/CriteriaDropdown.js @@ -3,8 +3,9 @@ import { Component } from 'react' import { Icon } from '../../../components' import * as PropTypes from 'prop-types' import Dropdown from '../../components/Dropdown' -import { MINOR, MAJOR, CRITICAL } from '../../utils/reject-trans-util' -import { UNSPECIFIED } from './index' +import { + MINOR, MAJOR, CRITICAL, UNSPECIFIED +} from '../../utils/reject-trans-util' /** * A Local Editor Dropdown coponent that selects the Criteria @@ -33,32 +34,24 @@ class CriteriaDropdown extends Component { })) } onCriteriaChange = (event) => { - this.props.onCriteriaChange(event) - this.toggleDropdown() - } - onUnspecifiedCriteria = () => { - this.props.onUnspecifiedCriteria() + if (event.target.innerText === UNSPECIFIED.description) { + this.props.onUnspecifiedCriteria() + } else { + this.props.onCriteriaChange(event) + } this.toggleDropdown() } render () { const { criteriaList, criteriaDescription } = this.props const options = criteriaList.map((value, index) => { return ( -
  • {value.description}
    • ) }) - // FIXME: should not be modifying the options array - options.unshift( -
  • - {UNSPECIFIED} -
    • - ) return ( {} const criteriaList = [{ @@ -22,22 +21,13 @@ const criteriaList = [{ }] const options = criteriaList.map((value, index) => { return ( -
  • {value.description}
    • ) }) - -// FIXME: should not be modifying the options array -options.unshift( -
  • - {UNSPECIFIED} -
    • -) /* global describe it expect */ describe('CriteriaDropdown', () => { it('renders default markup', () => { diff --git a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/RejectTranslationModal.story.js b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/RejectTranslationModal.story.js deleted file mode 100644 index e020981be8..0000000000 --- a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/RejectTranslationModal.story.js +++ /dev/null @@ -1,51 +0,0 @@ -// @ts-nocheck -import React from 'react' -import { storiesOf } from '@storybook/react' -import RejectTranslationModal from '.' -import Lorem from 'react-lorem-component' -import { MINOR, MAJOR, CRITICAL } from './index.js' - -/* eslint-disable max-len */ -/* - * TODO add stories showing the range of states - * for RejectTranslationModal - */ -storiesOf('RejectTranslationModal', module) - .addDecorator((story) => ( -
    -

    Lorem Ipsum

    - - -

    Dolor Sit Amet

    - - -
    - {story()} -
    -
    - )) - .add('Translation errors (critical)', () => ( - - )) - .add('Style Guide and Glossary Violations (minor)', () => ( - - )) - - .add('Other (major)', () => ( - - )) diff --git a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/index.js b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/index.js index 725275c794..89c319ccc6 100644 --- a/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/index.js +++ b/server/zanata-frontend/src/app/editor/containers/RejectTranslationModal/index.js @@ -12,10 +12,8 @@ import { rejectTranslation } from '../../actions/review-trans-actions' import update from 'immutability-helper' import { isUndefined, isEmpty } from 'lodash' import { - MINOR, MAJOR, CRITICAL, priorities, textState + MINOR, MAJOR, CRITICAL, UNSPECIFIED, priorities, textState } from '../../utils/reject-trans-util' - -export const UNSPECIFIED = 'Unspecified Criteria' const textLimit = 500 /** @@ -30,7 +28,7 @@ export class RejectTranslationModal extends Component { revision: PropTypes.number, localeId: PropTypes.string.isRequired, criteriaList: PropTypes.arrayOf(PropTypes.shape({ - id: PropTypes.number.isRequired, + id: PropTypes.number, editable: PropTypes.bool.isRequired, description: PropTypes.string.isRequired, priority: PropTypes.oneOf([MINOR, MAJOR, CRITICAL]).isRequired @@ -47,12 +45,7 @@ export class RejectTranslationModal extends Component { reviewComment: '' }, charsLeft: textLimit, - selectedCriteria: { - id: undefined, - editable: false, - description: '', - priority: MINOR - } + selectedCriteria: UNSPECIFIED } constructor (props) { super(props) @@ -81,14 +74,13 @@ export class RejectTranslationModal extends Component { })) } onUnspecifiedCriteria = () => { - const unspecifiedCriteria = this.defaultState.selectedCriteria this.setState(prevState => ({ review: update(prevState.review, { - criteriaDescription: {$set: UNSPECIFIED}, + criteriaDescription: {$set: UNSPECIFIED.description}, criteriaId: {$set: undefined}, - selectedPriority: {$set: unspecifiedCriteria.priority} + selectedPriority: {$set: UNSPECIFIED.priority} }), - selectedCriteria: unspecifiedCriteria + selectedCriteria: UNSPECIFIED })) } setReviewComment = (event) => { @@ -143,9 +135,12 @@ export class RejectTranslationModal extends Component { priorityChange={this.onPriorityChange} /> : undefined - const canReject = ( - (!isEmpty(review.reviewComment)) || - (selectedCriteria.editable === false)) + const cantReject = ( + (isEmpty(review.reviewComment)) && + (selectedCriteria.editable === true)) + const commentPlaceholder = (selectedCriteria.editable === true) + ? 'You must provide a comment for why this translation has been rejected' + : 'Provide a comment for why this translation has been rejected' return (