From 55641f62d017e25945ea7f5872c8940f1b14e69c Mon Sep 17 00:00:00 2001 From: Cagdas U Date: Sat, 7 Aug 2021 02:55:20 +0300 Subject: [PATCH 1/2] fix(roles): do not allow manual skill selection (if not in the list) * Fix an issue which was allowing users to be able to add non-existing skills. * The `Typeahead` component was also firing the `onChange` event on Blur, Enter and Esc presses. * To overcome this, added a property (`enforceListOnlySelection`) to the component. If it's set, the `onChange` event will be fired only if the input is in the suggestion list. * Add `minLengthForSuggestions` property to the `Typeahead` component. * This is the minimum length to retrieve & display suggestions and was hardcoded (with 3) in the component. * The new property allows to specify it. The ability to set it to `1` is useful for displaying skills like `C`, `C#`, which have less than 3 characters. Adresses https://github.com/topcoder-platform/taas-app/issues/426 --- src/components/Typeahead/index.jsx | 32 +++++++++++++------ .../Roles/components/RoleForm/index.jsx | 2 ++ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/components/Typeahead/index.jsx b/src/components/Typeahead/index.jsx index 9d71979..80c243c 100644 --- a/src/components/Typeahead/index.jsx +++ b/src/components/Typeahead/index.jsx @@ -74,6 +74,8 @@ const selectComponents = { * @param {function} props.onChange function called when value changes * @param {function} [props.onInputChange] function called when input value changes * @param {function} [props.onBlur] function called on input blur + * @param {number} [props.minLengthForSuggestions] the minimum string lenth for displaying suggestions (default 3) + * @param {Boolean} [props.enforceListOnlySelection] enforces user to select from the list - manual inputs (if not in the list) won't affect the selection * @param {string} props.value input value * @param {function} props.getSuggestions the function to get suggestions * @param {string} props.targetProp the target property of the returned object from getSuggestions @@ -87,6 +89,8 @@ const Typeahead = ({ onChange, onInputChange, onBlur, + minLengthForSuggestions = 3, + enforceListOnlySelection = false, placeholder, value, getSuggestions, @@ -139,7 +143,12 @@ const Typeahead = ({ setIsMenuFocused(false); setIsMenuOpen(false); setIsLoading(false); - onChange(inputValue); + // fire onChange event + // - if `enforceListOnlySelection` is not set, + // - or if it's set and options list contains the value + if (!enforceListOnlySelection || options.includes(inputValue)) { + onChange(inputValue); + } } } else if (key === "ArrowDown") { if (!isMenuFocused) { @@ -158,7 +167,12 @@ const Typeahead = ({ const onSelectBlur = () => { setIsMenuFocused(false); setIsMenuOpen(false); - onChange(inputValue); + // fire onChange event + // - if `enforceListOnlySelection` is not set, + // - or if it's set and options list contains the value + if (!enforceListOnlySelection || options.includes(inputValue)) { + onChange(inputValue); + } onBlur && onBlur(); }; @@ -170,11 +184,10 @@ const Typeahead = ({ } setIsLoading(true); setIsMenuOpen(true); - const options = await loadSuggestions( - getSuggestions, - value, - targetProp - ); + const options = + value.length < minLengthForSuggestions + ? [] // no suggestions yet if value length is less than `minLengthForSuggestions` + : await loadSuggestions(getSuggestions, value, targetProp); if (!isChangeAppliedRef.current) { setOptions(options); setIsLoading(false); @@ -233,9 +246,6 @@ const Typeahead = ({ const loadSuggestions = async (getSuggestions, inputValue, targetProp) => { let options = []; - if (inputValue.length < 3) { - return options; - } try { const res = await getSuggestions(inputValue); const items = res.data.slice(0, 100); @@ -266,6 +276,8 @@ Typeahead.propTypes = { onChange: PT.func.isRequired, onInputChange: PT.func, onBlur: PT.func, + minLengthForSuggestions: PT.number, + enforceListOnlySelection: PT.bool, placeholder: PT.string, value: PT.oneOfType([PT.number, PT.string]), getSuggestions: PT.func, diff --git a/src/routes/Roles/components/RoleForm/index.jsx b/src/routes/Roles/components/RoleForm/index.jsx index 70605f0..ed2e11d 100644 --- a/src/routes/Roles/components/RoleForm/index.jsx +++ b/src/routes/Roles/components/RoleForm/index.jsx @@ -194,8 +194,10 @@ function RoleForm() { id="skills" name="skills" placeholder="Search skills..." + enforceListOnlySelection={true} onChange={(val) => addSkill(val)} onInputChange={(val) => setTypeaheadInputValue(val)} + minLengthForSuggestions={1} // retrieve suggestions with min. 1 characters. Useful for skills like "C" value={typeaheadInputValue} getSuggestions={searchSkills} targetProp="name" From 6104e7dc5126d968072051a08cfd408a06c79f0a Mon Sep 17 00:00:00 2001 From: Cagdas U Date: Mon, 9 Aug 2021 07:50:28 +0300 Subject: [PATCH 2/2] feat(roles): allow manual inputs in integer fields * Remove +/- buttons in the integer fields of Roles form. * Allow manual input in the integer fields. * Add a logic to make time conversion between human-readable strings (e.g. "2 weeks") and numeric hours (336). * Add proper validations on manual inputs. Addresses: https://github.com/topcoder-platform/taas-app/issues/425, https://github.com/topcoder-platform/taas-app/issues/403#issuecomment-892774361 --- src/components/IntegerField/index.jsx | 127 ++++++-- .../IntegerField/styles.module.scss | 19 ++ .../Roles/components/RoleForm/index.jsx | 273 ++++++++++++------ .../components/RoleForm/styles.module.scss | 37 ++- src/routes/Roles/index.jsx | 44 ++- src/utils/misc.js | 60 ++++ 6 files changed, 432 insertions(+), 128 deletions(-) diff --git a/src/components/IntegerField/index.jsx b/src/components/IntegerField/index.jsx index 0af814f..e327ccb 100644 --- a/src/components/IntegerField/index.jsx +++ b/src/components/IntegerField/index.jsx @@ -1,6 +1,8 @@ -import React from "react"; +import React, { useMemo } from "react"; import PT from "prop-types"; import cn from "classnames"; +import IconExclamationMark from "components/Icons/ExclamationMarkCircled"; +import Popover from "components/Popover"; import styles from "./styles.module.scss"; /** @@ -9,58 +11,119 @@ import styles from "./styles.module.scss"; * @param {Object} props component properties * @param {string} [props.className] class name to be added to root element * @param {boolean} [props.isDisabled] if the field is disabled + * @param {boolean} [props.readOnly] if the field is readOnly + * @param {boolean} [props.displayButtons] whether to display +/- buttons * @param {string} props.name field's name * @param {number} props.value field's value * @param {number} [props.maxValue] maximum allowed value * @param {number} [props.minValue] minimum allowed value - * @param {(v: number) => void} props.onChange + * @param {(v: number) => void} [props.onChange] + * @param {(v: string) => void} [props.onInputChange] * @returns {JSX.Element} */ const IntegerField = ({ className, isDisabled = false, + readOnly = true, + displayButtons = true, name, + onInputChange, onChange, value, maxValue = Infinity, minValue = -Infinity, -}) => ( -
- -
-); +}) => { + const isInvalid = useMemo( + () => + !!value && + (isNaN(value) || + !Number.isInteger(+value) || + +value > maxValue || + +value < minValue), + [value, minValue, maxValue] + ); + + const errorPopupContent = useMemo(() => { + if (value && (isNaN(value) || !Number.isInteger(+value))) { + return <>You must enter a valid integer.; + } + if (+value > maxValue) { + return ( + <> + You must enter an integer less than or equal to{" "} + {maxValue}. + + ); + } + if (+value < minValue) { + return ( + <> + You must enter an integer greater than or equal to{" "} + {minValue}. + + ); + } + }, [value, minValue, maxValue]); + + return ( +
+ {isInvalid && ( + + + + )} + onInputChange && onInputChange(event.target.value)} + disabled={isDisabled} + readOnly={readOnly} + className={cn(styles.input, { + error: isInvalid, + })} + name={name} + value={value} + /> + {displayButtons && ( + <> +
+ ); +}; IntegerField.propTypes = { className: PT.string, isDisabled: PT.bool, + readOnly: PT.bool, + displayButtons: PT.bool, name: PT.string.isRequired, maxValue: PT.number, minValue: PT.number, - onChange: PT.func.isRequired, + onChange: PT.func, + onInputChange: PT.func, value: PT.number.isRequired, }; diff --git a/src/components/IntegerField/styles.module.scss b/src/components/IntegerField/styles.module.scss index 32cd095..0663b6f 100644 --- a/src/components/IntegerField/styles.module.scss +++ b/src/components/IntegerField/styles.module.scss @@ -19,6 +19,11 @@ input.input { outline: none !important; box-shadow: none !important; text-align: center; + appearance: textfield; + &::-webkit-outer-spin-button, &::-webkit-inner-spin-button { + -webkit-appearance: none; + margin: 0; + } &:disabled { border-color: $control-disabled-border-color; @@ -94,3 +99,17 @@ input.input { height: 9px; } } + +.popup { + margin-right: 5px; + max-width: 400px; + max-height: 200px; + line-height: $line-height-px; + white-space: normal; +} + +.icon { + padding-top: 1px; + width: 15px; + height: 15px; +} diff --git a/src/routes/Roles/components/RoleForm/index.jsx b/src/routes/Roles/components/RoleForm/index.jsx index ed2e11d..b4375b1 100644 --- a/src/routes/Roles/components/RoleForm/index.jsx +++ b/src/routes/Roles/components/RoleForm/index.jsx @@ -8,7 +8,10 @@ import cn from "classnames"; import { getRolesModalRole, getRolesModalError } from "store/selectors/roles"; import { setModalRole, setModalError } from "store/actions/roles"; import { searchSkills } from "services/roles"; +import { humanReadableTimeToHours } from "utils/misc"; import FallbackIcon from "../../../../assets/images/icon-role-fallback.svg"; +import IconExclamationMark from "components/Icons/ExclamationMarkCircled"; +import Popover from "components/Popover"; import Typeahead from "components/Typeahead"; import IntegerField from "components/IntegerField"; import IconArrowSmall from "components/Icons/ArrowSmall"; @@ -69,18 +72,18 @@ function RoleForm() { rates: [ ...roleState.rates, { - global: 0, - inCountry: 0, - offShore: 0, - niche: 0, - rate30Niche: 0, - rate30Global: 0, - rate30InCountry: 0, - rate30OffShore: 0, - rate20Niche: 0, - rate20Global: 0, - rate20InCountry: 0, - rate20OffShore: 0, + global: 10, + inCountry: 10, + offShore: 10, + niche: 10, + rate30Niche: 10, + rate30Global: 10, + rate30InCountry: 10, + rate30OffShore: 10, + rate20Niche: 10, + rate20Global: 10, + rate20InCountry: 10, + rate20OffShore: 10, }, ], }) @@ -89,14 +92,6 @@ function RoleForm() { const editRate = useCallback( (index, changes) => { - // a num field is `null` but user trying to increase/decrease it - // start with 0 - for (const key in changes) { - if (isNaN(changes[key])) { - changes[key] = 0; - } - } - // apply changes dispatch( setModalRole({ ...roleState, @@ -225,8 +220,11 @@ function RoleForm() { onChange({ numberOfMembers: num })} + onInputChange={(val) => onChange({ numberOfMembers: val })} />
@@ -234,33 +232,96 @@ function RoleForm() { onChange({ numberOfMembersAvailable: num })} + onInputChange={(val) => onChange({ numberOfMembersAvailable: val })} />
- Time to Candidate - onChange({ timeToCandidate: num })} - /> + Time to Interview +
+ {!!roleState.timeToInterview && + humanReadableTimeToHours(roleState.timeToInterview) === 0 && ( + + Example values: +
    +
  • 12 hours
    • +
  • 3 days
    • +
  • 2 weeks
    • +
  • 2 weeks 3 days
    • +
  • 1 month
    • +
  • ...
    • +
+ + } + strategy="fixed" + > + + + )} + + onChange({ + timeToInterview: event.target.value, + }) + } + /> +
- Time to Interview - onChange({ timeToInterview: num })} - /> + Time to Candidate +
+ {!!roleState.timeToCandidate && + humanReadableTimeToHours(roleState.timeToCandidate) === 0 && ( + + Example values: +
    +
  • 12 hours
    • +
  • 3 days
    • +
  • 2 weeks
    • +
  • 2 weeks 3 days
    • +
  • 1 month
    • +
  • ...
    • +
+ + } + strategy="fixed" + > + + + )} + + onChange({ + timeToCandidate: event.target.value, + }) + } + /> +