diff --git a/ISSUE_TEMPLATE.md b/ISSUE_TEMPLATE.md new file mode 100644 index 0000000..48c2d7a --- /dev/null +++ b/ISSUE_TEMPLATE.md @@ -0,0 +1,60 @@ +# What 👋 + +A _quick_ description outlining the context of this _issue_. + +### _Example:_ + +> Improve the current _login widget_ form validation. + +## Where 🔍 + +An overview of _"avenues"_ that are influenced/affected by this request. + +### _Example:_ + +> - This relates to the larger [#123](#) _WCAG 2 AA_ compliance issue. +> - The _login widget_ can be found on the core [`/login`](#) screen. + +## Why 🤔 + +Dispel any ambiguity around why this _issue_ needs a resolution. + +### _Example:_ + +> It has become a core requirement to the business that our high traffic pages get immediate treatment from an accessibility perspective. This is to ensure that we comply with the _WCAG 2 AA_ specifications and maintain our compliance rating. +> +> Currently our _login widget_ is falling short of an optimal _user_ experience and needs urgent attention given its significance to our application. + +## How 💡 + +Ideas/leads/breadcrumbs around how to proceed with resolving the _issue_. + +### _Example:_ + +> - [@jared](#) has put together a validation flow in which will act as a reference during the development phase. +> ![new-design](https://user-images.githubusercontent.com/15273233/52896073-95cf1280-3227-11e9-996d-3b9872f4f6c0.png) +> +> - [@sarah](#) is the _project owner_ for the _login widget_ and can help from a timing perspective. +> - [@tim](#) has recently added [`redux-form`](https://redux-form.com) to then _sign up_ page and says that it will be helpful in this scenario too. + +## Note 📋 + +Any information that does not fit into the above categories giving extra context to the _issue_. + +### _Example:_ + +> It would be nice at some stage to _pull_ these validation _"patterns"_ out into their own _global_ reference for everyone to use. In that regard, we can make a subsequent _issue_ that leverages this work as part of a refactor. + +## Demo 📺 + +Bring clarity to the _issue_ with visual aids: + +- **Screenshots:** `cmd` + `shift` + `4` _(MacOS)_. +- **Gifs:** [GIPHY Capture](https://giphy.com/apps/giphycapture) _(free/MacOS)_. +- **Code Snippets:** [Carbon](https://carbon.now.sh/) _(free)_. + +### _Example:_ + +> Validation message appears at the bottom of the widget and has no affiliation to the inputs that need addressing. The messaging is also ambiguous and offers **no context** to the _user_. +> +> ![form-before](https://user-images.githubusercontent.com/15273233/52890596-749c0100-31ea-11e9-94d4-588b914a4fde.gif) diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..19bb6c9 --- /dev/null +++ b/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,68 @@ +# What 👋 + +A _quick_ description outlining the context of this _pull request_. + +### _Example:_ + +> An update to the _client-side_ validation system for the _login widget_. + +## Where 🔍 + +An overview of _"avenues"_ that are influenced/affected by this work. + +### _Example:_ + +> - This resolves issue [`#123`](#) and [`#456`](#). +> - The _login widget_ can be found on the core [`/login`](#) screen. +> - Leverages the [`redux-form`](https://redux-form.com) implementation from pull request [`#789`](#). +> - This validation enhancement will hide behind feature flag `[LOGIN.VALIDATION]`. + +## Why 🤔 + +Dispel any ambiguity around why this bug/feature/enhancement was required. + +### _Example:_ + +> Although the current validation system worked from a technical perspective, there were concerns round _user_ accessibility _(specifically message location and content)_ which would impact our **WCAG 2 AA** compliance rating. + +## How 💡 + +Background on the changes/choices made to fulfill this _pull request_. + +### _Example:_ + +> - `redux-form` has a [built in validation system](https://redux-form.com/8.1.0/examples/syncvalidation/) that fits our needs. +> - The integration requires that our form `` elements conform to the [``](https://redux-form.com/8.1.0/docs/api/field.md/) abstraction _(which I created a simple HOC to achieve)_. +> - `redux-form` creates its own entry _(and format)_ in the `redux` _"store"_ so there were several references in the application that needed to be updated to the new state schema. + +## Note 📋 + +Any information that does not fit into the above categories giving extra context to the _pull request_. + +### _Example:_ + +> We endeavor to move this validation pattern into our [stand alone component architecture](#) next sprint. The _login widget_ is our initial test pilot _(to validate our validation enhancements)_. + +## Demo 📺 + +Bring clarity to the code with visual aids: + +- **Screenshots:** `cmd` + `shift` + `4` _(MacOS)_. +- **Gifs:** [GIPHY Capture](https://giphy.com/apps/giphycapture) _(free/MacOS)_. +- **Code Snippets:** [Carbon](https://carbon.now.sh/) _(free)_. + +If applicable, a **before** and **after** representation of your work is preferred. + +### _Example:_ + +> ### Before 👎 🙁 +> +> Global _invalidation_ message at the bottom of the form is visually discrete and uninformative. +> +> ![form-before](https://user-images.githubusercontent.com/15273233/52890596-749c0100-31ea-11e9-94d4-588b914a4fde.gif) +> +> ### After 👍 🙂 +> +> Individual _invalid_ messages on a per/input basis. Message _plus_ the `` itself has an error aesthetic. +> +> ![form-after](https://user-images.githubusercontent.com/15273233/52890599-7796f180-31ea-11e9-9b7b-af84a1107391.gif) diff --git a/package-lock.json b/package-lock.json index e692495..115886c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8210,11 +8210,6 @@ "resolved": "https://registry.npmjs.org/lodash._reinterpolate/-/lodash._reinterpolate-3.0.0.tgz", "integrity": "sha1-DM8tiRZq8Ds2Y8eWU4t1rG4RTZ0=" }, - "lodash.debounce": { - "version": "4.0.8", - "resolved": "https://registry.npmjs.org/lodash.debounce/-/lodash.debounce-4.0.8.tgz", - "integrity": "sha1-gteb/zCmfEAF/9XiUVMArZyk168=" - }, "lodash.memoize": { "version": "4.1.2", "resolved": "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-4.1.2.tgz", @@ -8242,11 +8237,6 @@ "lodash._reinterpolate": "^3.0.0" } }, - "lodash.throttle": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/lodash.throttle/-/lodash.throttle-4.1.1.tgz", - "integrity": "sha1-wj6RtxAkKscMN/HhzaknTMOb8vQ=" - }, "lodash.unescape": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/lodash.unescape/-/lodash.unescape-4.0.1.tgz", diff --git a/package.json b/package.json index 1438c1c..e5c8e68 100644 --- a/package.json +++ b/package.json @@ -10,8 +10,6 @@ "@fortawesome/free-solid-svg-icons": "^5.10.2", "@fortawesome/react-fontawesome": "^0.1.4", "drag-drop-touch": "^1.3.0", - "lodash.debounce": "^4.0.8", - "lodash.throttle": "^4.1.1", "nanoid": "^2.1.1", "normalize.css": "^8.0.1", "qs": "^6.9.1", diff --git a/src/App.js b/src/App.js index 46ff6d1..db10966 100644 --- a/src/App.js +++ b/src/App.js @@ -3,8 +3,8 @@ import "drag-drop-touch"; import React, { useCallback, useMemo, useRef, useState, useEffect } from "react"; import { TransitionGroup, CSSTransition } from "react-transition-group"; import nanoid from "nanoid"; -import throttle from "lodash.throttle"; import { createGlobalStyle } from "styled-components"; +import { useLoadControl } from "./LoadControl"; import { Swatches, UserSwatch, AppendSwatch } from "./Swatch"; import { Compositions, UserComposition, AppendComposition } from "./Composition"; import { Header } from "./Header"; @@ -104,49 +104,18 @@ const calculateReorderTransform = (swatches, dragStartId, dragOverId, swatchInde }; }; -const useThrottledState = (initialState, delay) => { - const [isPrepped, setIsPrepped] = useState(false); - const [state, setState] = useState(initialState); - const throttled = useRef(); - - useEffect(() => { - const handleUpdate = newState => setState(newState); - throttled.current = throttle(handleUpdate, delay, { trailing: false }); - - // Once the throttler has been setup we toggle a flag to ensure that we return - // the throttled state "updater" - if (!isPrepped) { - setIsPrepped(true); - } - - // Destroy persistent throttle reference on unmount. - return () => throttled.current.cancel(); - - // Force the effect to ONLY run on init so as NOT to recreate the thriller - // setup (which would be super bad). - }, []); - - return [ - state, - // If we have NOT prepped the throttler yet then just send back the "immediate" - // setState reference. - isPrepped ? throttled.current : setState - ]; -}; - const App = () => { /** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** SWATCHES: ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** ** **/ - // Se swap out the "vanilla" useState hook for a custom implementation that - // throttles the update of the "swatches" references. The swatch state is the - // catalyst for performant heavy re-renders (think hundreds of hex updates as - // you drag the native color slider). In that regard, we throttle the amount - // the swatches state can be updated. - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // const [swatches, setSwatches] = useState([]); - const [swatches, setSwatches] = useThrottledState(new Map([]), 1000); + const [swatches, setSwatches] = useState(new Map([])); + // We enrich the "vanilla" useState hook with a custom implementation that + // throttles and debounces the update of the "swatches" references. The swatch + // state is the catalyst for performant heavy re-renders (think hundreds of hex + // updates as you drag the native color slider). In that regard, we throttle + // the amount that the swatches state can be updated. + const setLoadControledSwatches = useLoadControl(setSwatches); const [dragStartId, setDragStartId] = useState(null); const [dragOverId, setDragOverId] = useState(null); @@ -160,7 +129,8 @@ const App = () => { removeDragOverId(null); }, [removeDragOverId]); - const appendSwatch = hex => setSwatches(new Map([...swatches, [createSwatchKey(), hex]])); + const appendSwatch = hex => + setLoadControledSwatches(swatches => new Map([...swatches, [createSwatchKey(), hex]])); const appendLastListedSwatch = () => { const [, lastHex] = [...swatches].pop() || []; @@ -173,7 +143,7 @@ const App = () => { }; const updateUserSwatch = useCallback( - (id, hex) => setSwatches(new Map([...swatches, [id, hex]])), + (id, hex) => setLoadControledSwatches(swatches => new Map([...swatches, [id, hex]])), [swatches] ); @@ -200,7 +170,7 @@ const App = () => { } }, []) ); - setSwatches(nextSwatches); + setLoadControledSwatches(nextSwatches); removeDragIds(); }, [swatches, dragStartId, removeDragIds] @@ -267,7 +237,7 @@ const App = () => { useEffect(() => { const { swatches, compositions } = convertStateFromQuery(window.location.search); - setSwatches(swatches); + setLoadControledSwatches(swatches); setCompositions(compositions); }, []); @@ -294,7 +264,7 @@ const App = () => { ...prevSwatches.slice(0, swatchIndex), ...prevSwatches.slice(swatchIndex + 1) ]; - setSwatches(new Map(nextSwatches)); + setLoadControledSwatches(new Map(nextSwatches)); }, [swatches] ); diff --git a/src/LoadControl.js b/src/LoadControl.js new file mode 100644 index 0000000..88eafdd --- /dev/null +++ b/src/LoadControl.js @@ -0,0 +1,69 @@ +import React, { useEffect, useRef } from "react"; + +export const LoadControl = (callback) => { + // Throttler - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // + let throttleId; + const createThrottle = (action) => (throttleId = window.requestAnimationFrame(action)); + const removeThrottle = () => (throttleId = window.cancelAnimationFrame(throttleId)); + const checkIsThrottling = () => Boolean(throttleId); + + // Debouncer - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // + const DEBOUNCE_MILLISECONDS = 100; + let debounceId; + const createDebounce = (...args) => + (debounceId = window.setTimeout(() => callback(...args), DEBOUNCE_MILLISECONDS)); + const removeDebounce = () => window.clearTimeout(debounceId); + + // Depending on the current "load controlled" situation we want to begin a + // throttle sequence or defer the callback to a debounced scenario. + const createLoadControl = () => (...args) => { + if (checkIsThrottling()) { + // If we are already throttling - the callback is STILL IMPORTANT. If the + // throttle finishes but misses the final user input then we could potential + // have the and UI out of sync. In this case we create + // a debounced, which will wait a period of time then run the supplied callback. + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // We do not want to stack callbacks and have them ALL run once their timeout + // expires. We ONLY care about the last supplied callback. In that regard, + // we destroy the preceding debounced setup and create a new one. This keep + // pushing out the time to run the callback while the thriller is still + // running. + removeDebounce(); + createDebounce(...args); + } else { + // If there is NO throttler instance then this is a "fresh" call to "load + // control". Here we run the callback inside of a requestAnimationCall so + // that its run when the browser has the capability to do so. + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // We ONLY want to run ONE callback per CPU cycle. In that regard we STOP + // callbacks from stacking by creating and removing the RAF reference so + // that ONLY one is running at a time BEFORE and AFTER the callback runs. + createThrottle(() => { + callback(...args); + removeThrottle(); + }); + } + }; + + const cleanUpLoadControl = () => { + removeThrottle(); + removeDebounce(); + }; + + return [createLoadControl, cleanUpLoadControl]; +}; + +export const useLoadControl = (callback) => { + const loadControl = useRef(); + + useEffect(() => { + const [createLoadControl, cleanUpLoadControl] = LoadControl(callback); + loadControl.current = createLoadControl(); + return cleanUpLoadControl; + }, []); + + // If the useEffect system has not been setup yet (happens in the first tick(s)) + // then we just fall back to the vanilla callback until the "load control" + // enrichment is complete. + return loadControl.current || callback; +}; diff --git a/src/Scroll.js b/src/Scroll.js index 8c1f555..884c32f 100644 --- a/src/Scroll.js +++ b/src/Scroll.js @@ -1,23 +1,26 @@ -import React, { useEffect, useRef } from "react"; -import throttle from "lodash.throttle"; +import React, { useEffect } from "react"; +import { LoadControl } from "./LoadControl"; export const Scroll = () => { - const offset = useRef(); - const throttledScroll = useRef(); - useEffect(() => { + let offset; + let throttleId; + const createThrottle = action => (throttleId = window.requestAnimationFrame(action)); + const removeThrottle = () => (throttleId = window.cancelAnimationFrame(throttleId)); + const checkIsThrottling = () => Boolean(throttleId); + const setScroll = () => { const { innerHeight, scrollY } = window; - const nextScroll = scrollY + offset.current; + const nextScroll = scrollY + offset; const isTooHigh = nextScroll < 0; const isTooLow = nextScroll > document.body.clientHeight - innerHeight; const shouldScroll = !isTooHigh && !isTooLow; if (shouldScroll) { window.scroll(0, nextScroll); - throttledScroll.current = requestAnimationFrame(setScroll); + createThrottle(setScroll); } else { - throttledScroll.current = null; + removeThrottle(); } }; @@ -28,43 +31,39 @@ export const Scroll = () => { const isOverTopQuarter = pointerPosition < viewPortQuarter; const isOverBottomQuarter = pointerPosition > viewPortQuarter * 3; const maxScrollOffset = viewPortQuarter; - const shouldUpdateScroll = - !throttledScroll.current && (isOverTopQuarter || isOverBottomQuarter); - const shouldStopScroll = - throttledScroll.current && !(isOverTopQuarter || isOverBottomQuarter); + const isThrottling = checkIsThrottling(); + const shouldUpdateScroll = !isThrottling && (isOverTopQuarter || isOverBottomQuarter); + const shouldStopScroll = isThrottling && !(isOverTopQuarter || isOverBottomQuarter); if (isOverTopQuarter) { const percentageOffset = (viewPortQuarter - pointerPosition) / viewPortQuarter; const pixelOffset = maxScrollOffset * percentageOffset; - offset.current = -pixelOffset; + offset = -pixelOffset; } if (isOverBottomQuarter) { const percentageOffset = (pointerPosition - viewPortQuarter * 3) / viewPortQuarter; const pixelOffset = maxScrollOffset * percentageOffset; - offset.current = pixelOffset; + offset = pixelOffset; } if (shouldUpdateScroll) { - throttledScroll.current = requestAnimationFrame(setScroll); + createThrottle(setScroll); } if (shouldStopScroll) { - throttledScroll.current = null; + removeThrottle(); } }; - const handlePointerMove = event => { - checkScenario(event); - }; - - const throttledDrag = throttle(handlePointerMove, 250, { trailing: false }); - - window.addEventListener("dragover", throttledDrag); + const [createDragLoadControl, cleanUpDragLoadControl] = LoadControl(checkScenario); + const dragLoadControl = createDragLoadControl(); + window.addEventListener("dragover", dragLoadControl); return function cleanUp() { - throttledScroll.current = null; - window.removeEventListener("dragover", handlePointerMove); + removeThrottle(); + cleanUpDragLoadControl(); + window.removeEventListener("dragover", dragLoadControl); }; }, []); diff --git a/src/Swatch.js b/src/Swatch.js index 46b8891..efb343c 100644 --- a/src/Swatch.js +++ b/src/Swatch.js @@ -1,9 +1,9 @@ import React, { memo, useState, useEffect, useRef, useCallback } from "react"; import { CSSTransition } from "react-transition-group"; import styled, { css } from "styled-components"; -import debounce from "lodash.debounce"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { faPlus, faTimes } from "@fortawesome/free-solid-svg-icons"; +import { useLoadControl } from "./LoadControl"; import { SWATCH_WIDTH, BORDER_WIDTH, @@ -34,7 +34,7 @@ import { checkHasLowLuminance, resetList, positionAbsolute, - deleteAnimation + deleteAnimation, } from "./utils"; const SwatchList = styled.ul` @@ -358,26 +358,25 @@ export const UserSwatch = memo( hasCapacityToDelete, createReorderTransform, shouldSwatchPronounce, - shouldSwatchRegress + shouldSwatchRegress, }) => { const [isDragged, setIsDragged] = useState(false); const [isAboutToDrag, setIsAboutToDrag] = useState(false); + const swatchRef = useRef(null); /** - * We are debouncing the color input change to our `swatch` global state. - * Debouncing causes the swatch hex to hang on the current value until the - * callback finally updates. This "hanging" makes the native color `` - * constantly revert back to the swatch hex rather than the users current - * selection. + * We are "load controling" the color input change to our `swatch` global state. + * Throttling/Debouncing causes the swatch hex to hang on the current value + * until the callback finally updates. This "hanging" makes the native color + * `` constantly revert back to the swatch hex rather than the users + * current selection. * * In that regard, we need to keep a local reference to what the user has * selected as their "next" hex choice so that the UI responds with a snappy * experience. */ const [inputValue, setInputValue] = useState(hex); - const debouncedInputHandler = debounce(value => handleChange(swatchId, value), 100); - - const swatchRef = useRef(null); + const handleLoadControledChange = useLoadControl((value) => handleChange(swatchId, value)); return ( { + onDragStart={(event) => { setIsAboutToDrag(false); /* * Even though we are setting the drag n drop state through React Firefox @@ -414,7 +413,7 @@ export const UserSwatch = memo( setIsDragged(false); handleDragEnd(); }} - onDragOver={event => { + onDragOver={(event) => { handleDragOver(swatchId); /* * MDN suggests applying `preventDefault` on specific DnD event hooks. @@ -423,7 +422,7 @@ export const UserSwatch = memo( event.preventDefault(); }} onDragLeave={handleDragExit} - onDrop={event => { + onDrop={(event) => { setIsDragged(false); handleDrop(swatchId); /* @@ -441,7 +440,8 @@ export const UserSwatch = memo( * listeners). In that regard when the main wrapper is "clicked" we give * focus to the nested . */ - swatchRef.current.querySelector("input").focus(); + const input = swatchRef.current.querySelector("input"); + input && input.focus(); }} > { + onChange={(event) => { const { value } = event.target; setInputValue(value); - debouncedInputHandler(value); + handleLoadControledChange(value); }} /> )} @@ -495,7 +495,7 @@ export const AppendSwatch = memo(({ dragHex, handleClick, handleDrop }) => { {...{ isTargeted }} hex={isTargeted && dragHex ? dragHex : GRAY_300} onClick={handleClick} - onDragOver={event => { + onDragOver={(event) => { /* * An `onDragOver` event MUST be present in order for a `onDrop` to * trigger! @@ -506,7 +506,7 @@ export const AppendSwatch = memo(({ dragHex, handleClick, handleDrop }) => { onDragLeave={() => setIsTargeted(false)} onPointerEnter={() => setIsTargeted(true)} onPointerLeave={() => setIsTargeted(false)} - onDrop={event => { + onDrop={(event) => { handleDrop(); setIsTargeted(false); /*