From 0f795392afa4864884ac0d2b5d4e6bde1bb21955 Mon Sep 17 00:00:00 2001 From: Mil4n0r Date: Mon, 19 Feb 2024 16:14:50 +0100 Subject: [PATCH 1/7] Fixed bug where text inputs were displaying NaN on scroll --- lib/src/text-input/TextInput.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/text-input/TextInput.tsx b/lib/src/text-input/TextInput.tsx index d5009480d..c5b532e03 100644 --- a/lib/src/text-input/TextInput.tsx +++ b/lib/src/text-input/TextInput.tsx @@ -249,7 +249,7 @@ const DxcTextInput = React.forwardRef( } }; const handleWheel = useCallback((event: WheelEvent) => { - if (document.activeElement === inputRef.current) { + if (document.activeElement === inputRef.current && numberInputContext?.typeNumber === "number") { event.preventDefault(); event.deltaY < 0 ? incrementNumber(inputRef.current.value) : decrementNumber(inputRef.current.value); } From 3299553f655d847e7c8b001f024c202491808c00 Mon Sep 17 00:00:00 2001 From: Mil4n0r Date: Tue, 20 Feb 2024 12:09:33 +0100 Subject: [PATCH 2/7] Prevent wheel listener from being created if condition not met --- lib/src/text-input/TextInput.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/src/text-input/TextInput.tsx b/lib/src/text-input/TextInput.tsx index c5b532e03..0ab2ffb3b 100644 --- a/lib/src/text-input/TextInput.tsx +++ b/lib/src/text-input/TextInput.tsx @@ -249,7 +249,7 @@ const DxcTextInput = React.forwardRef( } }; const handleWheel = useCallback((event: WheelEvent) => { - if (document.activeElement === inputRef.current && numberInputContext?.typeNumber === "number") { + if (document.activeElement === inputRef.current) { event.preventDefault(); event.deltaY < 0 ? incrementNumber(inputRef.current.value) : decrementNumber(inputRef.current.value); } @@ -350,9 +350,9 @@ const DxcTextInput = React.forwardRef( useEffect(() => { const input = inputRef.current; - - input.addEventListener('wheel', handleWheel, { passive: false }); - + if( numberInputContext?.typeNumber === "number") { + input.addEventListener('wheel', handleWheel, { passive: false }); + } return () => { input.removeEventListener('wheel', handleWheel); }; From 355e606634f0a66036f562f320ea980c77b9bd7a Mon Sep 17 00:00:00 2001 From: Mil4n0r Date: Wed, 21 Feb 2024 08:10:01 +0100 Subject: [PATCH 3/7] Applied condition to event listener removal --- lib/src/text-input/TextInput.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/src/text-input/TextInput.tsx b/lib/src/text-input/TextInput.tsx index 0ab2ffb3b..e96a0216c 100644 --- a/lib/src/text-input/TextInput.tsx +++ b/lib/src/text-input/TextInput.tsx @@ -350,12 +350,12 @@ const DxcTextInput = React.forwardRef( useEffect(() => { const input = inputRef.current; - if( numberInputContext?.typeNumber === "number") { - input.addEventListener('wheel', handleWheel, { passive: false }); + if (numberInputContext?.typeNumber === "number") { + input.addEventListener("wheel", handleWheel, { passive: false }); + return () => { + input.removeEventListener("wheel", handleWheel); + }; } - return () => { - input.removeEventListener('wheel', handleWheel); - }; }, [handleWheel]); return ( From c9b8b594acea033bd7604827ff18666818b110f2 Mon Sep 17 00:00:00 2001 From: Mil4n0r Date: Wed, 21 Feb 2024 08:11:55 +0100 Subject: [PATCH 4/7] Added modify functions to handleWheel dependencies --- lib/src/text-input/TextInput.tsx | 80 +++++++++++++++++--------------- 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/lib/src/text-input/TextInput.tsx b/lib/src/text-input/TextInput.tsx index e96a0216c..a0434d64c 100644 --- a/lib/src/text-input/TextInput.tsx +++ b/lib/src/text-input/TextInput.tsx @@ -168,6 +168,39 @@ const DxcTextInput = React.forwardRef( else onChange?.({ value: formattedValue }); }; + const decrementNumber = (currentValue = value ?? innerValue) => { + const numberValue = Number(currentValue); + const steppedValue = Math.round((numberValue - numberInputContext?.stepNumber + Number.EPSILON) * 100) / 100; + + if (currentValue !== "") { + if (numberValue < numberInputContext?.minNumber || steppedValue < numberInputContext?.minNumber) + changeValue(numberValue); + else if (numberValue > numberInputContext?.maxNumber) changeValue(numberInputContext?.maxNumber); + else if (numberValue === numberInputContext?.minNumber) changeValue(numberInputContext?.minNumber); + else changeValue(steppedValue); + } else { + if (numberInputContext?.minNumber >= 0) changeValue(numberInputContext?.minNumber); + else if (numberInputContext?.maxNumber < 0) changeValue(numberInputContext?.maxNumber); + else changeValue(-numberInputContext.stepNumber); + } + }; + const incrementNumber = (currentValue = value ?? innerValue) => { + const numberValue = Number(currentValue); + const steppedValue = Math.round((numberValue + numberInputContext?.stepNumber + Number.EPSILON) * 100) / 100; + + if (currentValue !== "") { + if (numberValue > numberInputContext?.maxNumber || steppedValue > numberInputContext?.maxNumber) + changeValue(numberValue); + else if (numberValue < numberInputContext?.minNumber) changeValue(numberInputContext?.minNumber); + else if (numberValue === numberInputContext?.maxNumber) changeValue(numberInputContext?.maxNumber); + else changeValue(steppedValue); + } else { + if (numberInputContext?.minNumber > 0) changeValue(numberInputContext?.minNumber); + else if (numberInputContext?.maxNumber <= 0) changeValue(numberInputContext?.maxNumber); + else changeValue(numberInputContext.stepNumber); + } + }; + const handleInputContainerOnClick = () => { document.activeElement !== actionRef.current && inputRef.current.focus(); }; @@ -248,12 +281,15 @@ const DxcTextInput = React.forwardRef( break; } }; - const handleWheel = useCallback((event: WheelEvent) => { - if (document.activeElement === inputRef.current) { - event.preventDefault(); - event.deltaY < 0 ? incrementNumber(inputRef.current.value) : decrementNumber(inputRef.current.value); - } - }, []); + const handleWheel = useCallback( + (event: WheelEvent) => { + if (document.activeElement === inputRef.current) { + event.preventDefault(); + event.deltaY < 0 ? incrementNumber(inputRef.current.value) : decrementNumber(inputRef.current.value); + } + }, + [incrementNumber, decrementNumber] + ); const handleClearActionOnClick = () => { changeValue(""); @@ -276,38 +312,6 @@ const DxcTextInput = React.forwardRef( inputRef?.current?.setAttribute("step", step); inputRef?.current?.setAttribute("type", type); }; - const decrementNumber = (currentValue = value ?? innerValue) => { - const numberValue = Number(currentValue); - const steppedValue = Math.round((numberValue - numberInputContext?.stepNumber + Number.EPSILON) * 100) / 100; - - if (currentValue !== "") { - if (numberValue < numberInputContext?.minNumber || steppedValue < numberInputContext?.minNumber) - changeValue(numberValue); - else if (numberValue > numberInputContext?.maxNumber) changeValue(numberInputContext?.maxNumber); - else if (numberValue === numberInputContext?.minNumber) changeValue(numberInputContext?.minNumber); - else changeValue(steppedValue); - } else { - if (numberInputContext?.minNumber >= 0) changeValue(numberInputContext?.minNumber); - else if (numberInputContext?.maxNumber < 0) changeValue(numberInputContext?.maxNumber); - else changeValue(-numberInputContext.stepNumber); - } - }; - const incrementNumber = (currentValue = value ?? innerValue) => { - const numberValue = Number(currentValue); - const steppedValue = Math.round((numberValue + numberInputContext?.stepNumber + Number.EPSILON) * 100) / 100; - - if (currentValue !== "") { - if (numberValue > numberInputContext?.maxNumber || steppedValue > numberInputContext?.maxNumber) - changeValue(numberValue); - else if (numberValue < numberInputContext?.minNumber) changeValue(numberInputContext?.minNumber); - else if (numberValue === numberInputContext?.maxNumber) changeValue(numberInputContext?.maxNumber); - else changeValue(steppedValue); - } else { - if (numberInputContext?.minNumber > 0) changeValue(numberInputContext?.minNumber); - else if (numberInputContext?.maxNumber <= 0) changeValue(numberInputContext?.maxNumber); - else changeValue(numberInputContext.stepNumber); - } - }; useEffect(() => { if (typeof suggestions === "function") { From 25eb1f1125ffc900280a10c4a9dfbd055810d36b Mon Sep 17 00:00:00 2001 From: Mil4n0r Date: Wed, 21 Feb 2024 10:11:14 +0100 Subject: [PATCH 5/7] Added wheel testing for text and number inputs --- lib/src/number-input/NumberInput.test.js | 82 +++++++++++++++++++++++- lib/src/text-input/TextInput.test.js | 11 ++++ lib/src/text-input/TextInput.tsx | 56 ++++++++-------- 3 files changed, 121 insertions(+), 28 deletions(-) diff --git a/lib/src/number-input/NumberInput.test.js b/lib/src/number-input/NumberInput.test.js index dd337b081..f186c5ba1 100644 --- a/lib/src/number-input/NumberInput.test.js +++ b/lib/src/number-input/NumberInput.test.js @@ -49,7 +49,7 @@ describe("Number input component tests", () => { await userEvent.click(increment); expect(number.value).toBe(""); }); - + test("Number input is read only and cannot be incremented or decremented using the arrow keys", () => { const { getByLabelText } = render(); const number = getByLabelText("Number label"); @@ -68,7 +68,9 @@ describe("Number input component tests", () => { test("Number input is not optional: required field, displays error if not filled in", () => { const onBlur = jest.fn(); const onChange = jest.fn(); - const { getByLabelText } = render(); + const { getByLabelText } = render( + + ); const number = getByLabelText("Number input label"); userEvent.type(number, "1"); userEvent.clear(number); @@ -381,6 +383,82 @@ describe("Number input component tests", () => { expect(number.value).toBe("5"); }); + test("Value is unchanged when using the scroll wheel in mouse in a disabled input", () => { + const { getByLabelText } = render( ); + const number = getByLabelText("Number input label"); + fireEvent.wheel(number, { deltaY: -100 }); + expect(number.value).toBe("10"); + fireEvent.wheel(number, { deltaY: 100 }); + expect(number.value).toBe("10"); + fireEvent.wheel(number, { deltaY: -100 }); + expect(number.value).toBe("10"); + fireEvent.wheel(number, { deltaY: 100 }); + expect(number.value).toBe("10"); + }); + + test("Value is unchanged when using the arrows in keyboard in a disabled input", () => { + const { getByLabelText } = render( ); + const number = getByLabelText("Number input label"); + fireEvent.keyDown(number, { keyCode: 38 }); + expect(number.value).toBe("10"); + fireEvent.keyDown(number, { keyCode: 40 }); + expect(number.value).toBe("10"); + fireEvent.keyDown(number, { keyCode: 38 }); + expect(number.value).toBe("10"); + fireEvent.keyDown(number, { keyCode: 40 }); + expect(number.value).toBe("10"); + }); + + test("Value is unchanged when using the scroll wheel in mouse in a read-only input", () => { + const { getByLabelText } = render( ); + const number = getByLabelText("Number input label"); + fireEvent.wheel(number, { deltaY: -100 }); + expect(number.value).toBe("10"); + fireEvent.wheel(number, { deltaY: 100 }); + expect(number.value).toBe("10"); + fireEvent.wheel(number, { deltaY: -100 }); + expect(number.value).toBe("10"); + fireEvent.wheel(number, { deltaY: 100 }); + expect(number.value).toBe("10"); + }); + + test("Value is unchanged when using the arrows in keyboard in a read-only input", () => { + const { getByLabelText } = render( ); + const number = getByLabelText("Number input label"); + fireEvent.keyDown(number, { keyCode: 38 }); + expect(number.value).toBe("10"); + fireEvent.keyDown(number, { keyCode: 40 }); + expect(number.value).toBe("10"); + fireEvent.keyDown(number, { keyCode: 38 }); + expect(number.value).toBe("10"); + fireEvent.keyDown(number, { keyCode: 40 }); + expect(number.value).toBe("10"); + }); + + test("Increment and decrement the value with min, max and step using the scroll wheel in mouse", () => { + const { getByLabelText } = render(); + const number = getByLabelText("Number input label"); + userEvent.type(number, "1"); + fireEvent.wheel(number, { deltaY: -100 }); + expect(number.value).toBe("5"); + fireEvent.wheel(number, { deltaY: -100 }); + expect(number.value).toBe("10"); + fireEvent.wheel(number, { deltaY: -100 }); + expect(number.value).toBe("15"); + fireEvent.wheel(number, { deltaY: -100 }); + expect(number.value).toBe("20"); + fireEvent.wheel(number, { deltaY: -100 }); + expect(number.value).toBe("20"); + fireEvent.wheel(number, { deltaY: 100 }); + expect(number.value).toBe("15"); + fireEvent.wheel(number, { deltaY: 100 }); + expect(number.value).toBe("10"); + fireEvent.wheel(number, { deltaY: 100 }); + expect(number.value).toBe("5"); + fireEvent.wheel(number, { deltaY: 100 }); + expect(number.value).toBe("5"); + }); + test("Number has correct accessibility attributes", () => { const { getByLabelText, getAllByRole } = render(); const number = getByLabelText("Number input label"); diff --git a/lib/src/text-input/TextInput.test.js b/lib/src/text-input/TextInput.test.js index e3588c084..4c8988903 100644 --- a/lib/src/text-input/TextInput.test.js +++ b/lib/src/text-input/TextInput.test.js @@ -462,6 +462,17 @@ describe("TextInput component tests", () => { const options = getAllByRole("option"); expect(options[0].getAttribute("aria-selected")).toBeNull(); }); + + test("Mouse wheel interaction does not affect the text value", () => { + const { getByRole } = render( + + ); + const input = getByRole("textbox"); + fireEvent.wheel(input, { deltaY: -100 }); + expect(input.value).toBe("Example text"); + fireEvent.wheel(input, { deltaY: 100 }); + expect(input.value).toBe("Example text"); + }); }); describe("TextInput component synchronous autosuggest tests", () => { diff --git a/lib/src/text-input/TextInput.tsx b/lib/src/text-input/TextInput.tsx index a0434d64c..94dc3fef4 100644 --- a/lib/src/text-input/TextInput.tsx +++ b/lib/src/text-input/TextInput.tsx @@ -169,35 +169,39 @@ const DxcTextInput = React.forwardRef( }; const decrementNumber = (currentValue = value ?? innerValue) => { - const numberValue = Number(currentValue); - const steppedValue = Math.round((numberValue - numberInputContext?.stepNumber + Number.EPSILON) * 100) / 100; - - if (currentValue !== "") { - if (numberValue < numberInputContext?.minNumber || steppedValue < numberInputContext?.minNumber) - changeValue(numberValue); - else if (numberValue > numberInputContext?.maxNumber) changeValue(numberInputContext?.maxNumber); - else if (numberValue === numberInputContext?.minNumber) changeValue(numberInputContext?.minNumber); - else changeValue(steppedValue); - } else { - if (numberInputContext?.minNumber >= 0) changeValue(numberInputContext?.minNumber); - else if (numberInputContext?.maxNumber < 0) changeValue(numberInputContext?.maxNumber); - else changeValue(-numberInputContext.stepNumber); + if (!disabled && !readOnly) { + const numberValue = Number(currentValue); + const steppedValue = Math.round((numberValue - numberInputContext?.stepNumber + Number.EPSILON) * 100) / 100; + + if (currentValue !== "") { + if (numberValue < numberInputContext?.minNumber || steppedValue < numberInputContext?.minNumber) + changeValue(numberValue); + else if (numberValue > numberInputContext?.maxNumber) changeValue(numberInputContext?.maxNumber); + else if (numberValue === numberInputContext?.minNumber) changeValue(numberInputContext?.minNumber); + else changeValue(steppedValue); + } else { + if (numberInputContext?.minNumber >= 0) changeValue(numberInputContext?.minNumber); + else if (numberInputContext?.maxNumber < 0) changeValue(numberInputContext?.maxNumber); + else changeValue(-numberInputContext.stepNumber); + } } }; const incrementNumber = (currentValue = value ?? innerValue) => { - const numberValue = Number(currentValue); - const steppedValue = Math.round((numberValue + numberInputContext?.stepNumber + Number.EPSILON) * 100) / 100; - - if (currentValue !== "") { - if (numberValue > numberInputContext?.maxNumber || steppedValue > numberInputContext?.maxNumber) - changeValue(numberValue); - else if (numberValue < numberInputContext?.minNumber) changeValue(numberInputContext?.minNumber); - else if (numberValue === numberInputContext?.maxNumber) changeValue(numberInputContext?.maxNumber); - else changeValue(steppedValue); - } else { - if (numberInputContext?.minNumber > 0) changeValue(numberInputContext?.minNumber); - else if (numberInputContext?.maxNumber <= 0) changeValue(numberInputContext?.maxNumber); - else changeValue(numberInputContext.stepNumber); + if (!disabled && !readOnly) { + const numberValue = Number(currentValue); + const steppedValue = Math.round((numberValue + numberInputContext?.stepNumber + Number.EPSILON) * 100) / 100; + + if (currentValue !== "") { + if (numberValue > numberInputContext?.maxNumber || steppedValue > numberInputContext?.maxNumber) + changeValue(numberValue); + else if (numberValue < numberInputContext?.minNumber) changeValue(numberInputContext?.minNumber); + else if (numberValue === numberInputContext?.maxNumber) changeValue(numberInputContext?.maxNumber); + else changeValue(steppedValue); + } else { + if (numberInputContext?.minNumber > 0) changeValue(numberInputContext?.minNumber); + else if (numberInputContext?.maxNumber <= 0) changeValue(numberInputContext?.maxNumber); + else changeValue(numberInputContext.stepNumber); + } } }; From ca688411d105cbe759786db3ffc5316437d3c8c8 Mon Sep 17 00:00:00 2001 From: Mil4n0r Date: Thu, 22 Feb 2024 08:17:50 +0100 Subject: [PATCH 6/7] Prettier formating --- lib/src/number-input/NumberInput.test.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/src/number-input/NumberInput.test.js b/lib/src/number-input/NumberInput.test.js index f186c5ba1..ace5a0d58 100644 --- a/lib/src/number-input/NumberInput.test.js +++ b/lib/src/number-input/NumberInput.test.js @@ -384,7 +384,9 @@ describe("Number input component tests", () => { }); test("Value is unchanged when using the scroll wheel in mouse in a disabled input", () => { - const { getByLabelText } = render( ); + const { getByLabelText } = render( + + ); const number = getByLabelText("Number input label"); fireEvent.wheel(number, { deltaY: -100 }); expect(number.value).toBe("10"); @@ -397,7 +399,9 @@ describe("Number input component tests", () => { }); test("Value is unchanged when using the arrows in keyboard in a disabled input", () => { - const { getByLabelText } = render( ); + const { getByLabelText } = render( + + ); const number = getByLabelText("Number input label"); fireEvent.keyDown(number, { keyCode: 38 }); expect(number.value).toBe("10"); @@ -410,7 +414,9 @@ describe("Number input component tests", () => { }); test("Value is unchanged when using the scroll wheel in mouse in a read-only input", () => { - const { getByLabelText } = render( ); + const { getByLabelText } = render( + + ); const number = getByLabelText("Number input label"); fireEvent.wheel(number, { deltaY: -100 }); expect(number.value).toBe("10"); @@ -423,7 +429,9 @@ describe("Number input component tests", () => { }); test("Value is unchanged when using the arrows in keyboard in a read-only input", () => { - const { getByLabelText } = render( ); + const { getByLabelText } = render( + + ); const number = getByLabelText("Number input label"); fireEvent.keyDown(number, { keyCode: 38 }); expect(number.value).toBe("10"); From 28cb733cef56454ddb3047a87d8e149041192522 Mon Sep 17 00:00:00 2001 From: Mil4n0r Date: Mon, 26 Feb 2024 09:02:42 +0100 Subject: [PATCH 7/7] Changed wheel event handling inside NumberInput --- lib/src/number-input/NumberInput.tsx | 72 +++++++++++++++++----------- lib/src/text-input/TextInput.tsx | 24 ++-------- 2 files changed, 49 insertions(+), 47 deletions(-) diff --git a/lib/src/number-input/NumberInput.tsx b/lib/src/number-input/NumberInput.tsx index af4122fe0..91057a2d1 100644 --- a/lib/src/number-input/NumberInput.tsx +++ b/lib/src/number-input/NumberInput.tsx @@ -1,4 +1,4 @@ -import React from "react"; +import React, { useEffect } from "react"; import styled from "styled-components"; import DxcTextInput from "../text-input/TextInput"; import NumberInputPropsType, { RefType } from "./types"; @@ -38,33 +38,49 @@ const DxcNumberInput = React.forwardRef( tabIndex, }, ref - ) => ( - - - - - - ) + ) => { + const numberInputRef = React.useRef(null); + + useEffect(() => { + const input = numberInputRef.current?.getElementsByTagName("input")[0] as HTMLInputElement; + const preventDefault = (event: WheelEvent) => { + event.preventDefault(); + }; + + input?.addEventListener("wheel", preventDefault, { passive: false }); + return () => { + input?.removeEventListener("wheel", preventDefault); + }; + }, []); + + return ( + + + + + + ); + } ); const NumberInputContainer = styled.div` diff --git a/lib/src/text-input/TextInput.tsx b/lib/src/text-input/TextInput.tsx index 94dc3fef4..58b429782 100644 --- a/lib/src/text-input/TextInput.tsx +++ b/lib/src/text-input/TextInput.tsx @@ -285,15 +285,10 @@ const DxcTextInput = React.forwardRef( break; } }; - const handleWheel = useCallback( - (event: WheelEvent) => { - if (document.activeElement === inputRef.current) { - event.preventDefault(); - event.deltaY < 0 ? incrementNumber(inputRef.current.value) : decrementNumber(inputRef.current.value); - } - }, - [incrementNumber, decrementNumber] - ); + const handleNumberInputWheel = (event: React.WheelEvent) => { + if (document.activeElement === inputRef.current) + event.deltaY < 0 ? incrementNumber(inputRef.current.value) : decrementNumber(inputRef.current.value); + }; const handleClearActionOnClick = () => { changeValue(""); @@ -356,16 +351,6 @@ const DxcTextInput = React.forwardRef( ); }, [value, innerValue, suggestions, numberInputContext]); - useEffect(() => { - const input = inputRef.current; - if (numberInputContext?.typeNumber === "number") { - input.addEventListener("wheel", handleWheel, { passive: false }); - return () => { - input.removeEventListener("wheel", handleWheel); - }; - } - }, [handleWheel]); - return ( @@ -436,6 +421,7 @@ const DxcTextInput = React.forwardRef( onMouseDown={(event) => { event.stopPropagation(); }} + onWheel={numberInputContext?.typeNumber === "number" ? handleNumberInputWheel : undefined} disabled={disabled} readOnly={readOnly} ref={inputRef}