From c413337de14fc84e03db474fbb95453e359ba98a Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Mon, 18 Dec 2023 14:56:47 +0100 Subject: [PATCH 01/12] Handle copy-paste and removal of full minutes and hours --- src/components/TimePicker/TimePicker.js | 40 +++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/src/components/TimePicker/TimePicker.js b/src/components/TimePicker/TimePicker.js index 5b49739150cc..53f2e59c424d 100644 --- a/src/components/TimePicker/TimePicker.js +++ b/src/components/TimePicker/TimePicker.js @@ -114,14 +114,30 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { [hours, minute, amPmValue, defaultValue], ); + const resetHours = () => { + setHours('00'); + setSelectionHour({start: 0, end: 0}); + } + + const resetMinutes = () => { + setMinute('00'); + setSelectionMinute({start: 0, end: 0}); + } + // This function receive value from hour input and validate it // The valid format is HH(from 00 to 12). If the user input 9, it will be 09. If user try to change 09 to 19 it would skip the first character const handleHourChange = (text) => { + if (_.isEmpty(text)) { + resetHours(); + return; + } + const isOnlyNumericValue = /^\d+$/.test(text.trim()); // Skip if the user is pasting the text or use non numeric characters. if (selectionHour.start !== selectionHour.end || !isOnlyNumericValue) { return; } + // Remove non-numeric characters. const filteredText = text.replace(/[^0-9]/g, ''); @@ -186,6 +202,12 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { // This function receive value from minute input and validate it // The valid format is MM(from 00 to 59). If the user input 9, it will be 09. If user try to change 09 to 99 it would skip the character const handleMinutesChange = (text) => { + if (_.isEmpty(text)) { + resetMinutes(); + focusHourInputOnLastCharacter(); + return; + } + const isOnlyNumericValue = /^\d+$/.test(text.trim()); // Skip if the user is pasting the text or use non numeric characters. if (selectionMinute.start !== selectionMinute.end || !isOnlyNumericValue) { @@ -262,14 +284,26 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } if (key === '<' || key === 'Backspace') { if (isHourFocused) { + if (selectionHour.start === 0 && selectionHour.end === 2) { + resetHours(); + return; + } + const newHour = replaceWithZeroAtPosition(hours, selectionHour.start); setHours(newHour); setSelectionHour(decreaseBothSelectionByOne(selectionHour)); } else if (isMinuteFocused) { - if (selectionMinute.start === 0) { + if (selectionMinute.start === 0 && selectionMinute.end === 2) { + resetMinutes(); focusHourInputOnLastCharacter(); return; } + + if (selectionMinute.start === 0 && selectionMinute.end === 0) { + focusHourInputOnLastCharacter(); + return; + } + const newMinute = replaceWithZeroAtPosition(minute, selectionMinute.start); setMinute(newMinute); setSelectionMinute(decreaseBothSelectionByOne(selectionMinute)); @@ -322,13 +356,13 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { const handleFocusOnBackspace = useCallback( (e) => { - if (selectionMinute.start !== 0 || e.key !== 'Backspace') { + if (selectionMinute.start !== 0 || selectionMinute.end !== 0 || e.key !== 'Backspace') { return; } hourInputRef.current.focus(); }, // eslint-disable-next-line react-hooks/exhaustive-deps - [selectionMinute.start], + [selectionMinute.start, selectionMinute.end], ); const {styleForAM, styleForPM} = StyleUtils.getStatusAMandPMButtonStyle(amPmValue); From d2cac37ce493194c13d7aeced2ac99625f47a5c4 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Wed, 20 Dec 2023 12:58:14 +0100 Subject: [PATCH 02/12] Support full selection replacement --- src/components/TimePicker/TimePicker.js | 75 +++++++++++++------------ 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/src/components/TimePicker/TimePicker.js b/src/components/TimePicker/TimePicker.js index 53f2e59c424d..038382562dda 100644 --- a/src/components/TimePicker/TimePicker.js +++ b/src/components/TimePicker/TimePicker.js @@ -117,12 +117,13 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { const resetHours = () => { setHours('00'); setSelectionHour({start: 0, end: 0}); - } + }; const resetMinutes = () => { setMinute('00'); setSelectionMinute({start: 0, end: 0}); - } + focusHourInputOnLastCharacter(); + }; // This function receive value from hour input and validate it // The valid format is HH(from 00 to 12). If the user input 9, it will be 09. If user try to change 09 to 19 it would skip the first character @@ -133,8 +134,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } const isOnlyNumericValue = /^\d+$/.test(text.trim()); - // Skip if the user is pasting the text or use non numeric characters. - if (selectionHour.start !== selectionHour.end || !isOnlyNumericValue) { + if (!isOnlyNumericValue) { return; } @@ -144,13 +144,11 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { let newHour = hours; let newSelection = selectionHour.start; - // Case when the cursor is at the start. - if (selectionHour.start === 0) { - // Handle cases where the hour would be > 12. - - // when you entering text the filteredText would consist of three numbers + if (selectionHour.start === 0 && selectionHour.end === 0) { + // When the user is entering text, the filteredText consists of three numbers const formattedText = `${filteredText[0]}${filteredText[2] || 0}`; if (formattedText > 12 && formattedText <= 24) { + // The hour is between 12 and 24 – switch AM to PM. newHour = String(formattedText - 12).padStart(2, '0'); newSelection = 2; setAmPmValue(CONST.TIME_PERIOD.PM); @@ -161,8 +159,8 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newHour = `${formattedText[0]}${formattedText[1]}`; newSelection = 1; } - } else if (selectionHour.start === 1) { - // Case when the cursor is at the second position. + } else if (selectionHour.start === 1 && selectionHour.end === 1) { + // The cursor is at the second position. const formattedText = `${filteredText[0]}${filteredText[1]}`; if (filteredText.length < 2) { @@ -183,13 +181,25 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newSelection = 2; } } else if (selectionHour.start === 2 && selectionHour.end === 2) { - // Case when the cursor is at the end and no text is selected. + // The cursor is at the end and no text is selected. if (filteredText.length < 2) { newHour = `${text}0`; newSelection = 1; } else { newSelection = 2; } + } else if (selectionHour.start === 0 && selectionHour.end === 2) { + if (filteredText <= 1) { + newHour = `${filteredText}0`; + newSelection = 1; + } else if (filteredText > 12 && filteredText <= 24) { + newHour = String(filteredText - 12).padStart(2, '0'); + newSelection = 2; + setAmPmValue(CONST.TIME_PERIOD.PM); + } else { + newHour = String(Math.min(filteredText, 12)).padStart(2, '0'); + newSelection = 2; + } } setHours(newHour); @@ -204,13 +214,11 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { const handleMinutesChange = (text) => { if (_.isEmpty(text)) { resetMinutes(); - focusHourInputOnLastCharacter(); return; } const isOnlyNumericValue = /^\d+$/.test(text.trim()); - // Skip if the user is pasting the text or use non numeric characters. - if (selectionMinute.start !== selectionMinute.end || !isOnlyNumericValue) { + if (!isOnlyNumericValue) { return; } @@ -219,22 +227,9 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { let newMinute = minute; let newSelection = selectionMinute.start; - // Case when user selects and replaces the text. - if (selectionMinute.start !== selectionMinute.end) { - // If the first digit is > 5, prepend 0. - if (filteredText.length === 1 && filteredText > 5) { - newMinute = `0${filteredText}`; - newSelection = 2; - // If the first digit is <= 5, append 0 at the end. - } else if (filteredText.length === 1 && filteredText <= 5) { - newMinute = `${filteredText}0`; - newSelection = 1; - } else { - newMinute = `${filteredText.slice(0, 2)}`; - newSelection = 2; - } - } else if (selectionMinute.start === 0) { - // Case when the cursor is at the start. + + if (selectionMinute.start === 0 && selectionMinute.end === 0) { + // The cursor is at the start. const formattedText = `${filteredText[0]}${filteredText[2] || 0}`; if (text[0] >= 6) { newMinute = `0${formattedText[1]}`; @@ -243,8 +238,8 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newMinute = `${formattedText[0]}${formattedText[1]}`; newSelection = 1; } - } else if (selectionMinute.start === 1) { - // Case when the cursor is at the second position. + } else if (selectionMinute.start === 1 && selectionMinute.end === 1) { + // The cursor is at the second position. // If we remove a value, prepend 0. if (filteredText.length < 2) { newMinute = `0${text}`; @@ -255,10 +250,19 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newMinute = `${text[0]}${text[1]}`; newSelection = 2; } - } else if (filteredText.length < 2) { - // Case when the cursor is at the end and no text is selected. + } else if (selectionMinute.start === 2 && selectionMinute.end === 2 && filteredText.length < 2) { + // The cursor is at the end and no text is selected newMinute = `${text}0`; newSelection = 1; + } else if (selectionMinute.start === 0 && selectionMinute.end === 2) { + // The user selects and replaces the text + if (filteredText <= 5) { + newMinute = `${filteredText}0`; + newSelection = 1; + } else { + newMinute = String(Math.min(filteredText, 59)).padStart(2, '0'); + newSelection = 2; + } } setMinute(newMinute); @@ -295,7 +299,6 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } else if (isMinuteFocused) { if (selectionMinute.start === 0 && selectionMinute.end === 2) { resetMinutes(); - focusHourInputOnLastCharacter(); return; } From 75292ecedce99666743f05ecf1c5ae2a304d6f63 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Wed, 20 Dec 2023 20:57:24 +0100 Subject: [PATCH 03/12] Support full selection replacement --- src/components/TimePicker/TimePicker.js | 64 ++++++++++--------------- 1 file changed, 24 insertions(+), 40 deletions(-) diff --git a/src/components/TimePicker/TimePicker.js b/src/components/TimePicker/TimePicker.js index 038382562dda..2ae364de4c26 100644 --- a/src/components/TimePicker/TimePicker.js +++ b/src/components/TimePicker/TimePicker.js @@ -165,7 +165,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { if (filteredText.length < 2) { // If we remove a value, prepend 0. - newHour = `0${text}`; + newHour = `0${filteredText}`; newSelection = 0; // If the second digit is > 2, replace the hour with 0 and the second digit. } else if (formattedText > 12 && formattedText <= 24) { @@ -173,33 +173,23 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newSelection = 2; setAmPmValue(CONST.TIME_PERIOD.PM); } else if (formattedText > 24) { - newHour = `0${text[1]}`; + newHour = `0${filteredText[1]}`; newSelection = 2; } else { - newHour = `${text[0]}${text[1]}`; + newHour = `${filteredText[0]}${filteredText[1]}`; setHours(newHour); newSelection = 2; } - } else if (selectionHour.start === 2 && selectionHour.end === 2) { - // The cursor is at the end and no text is selected. - if (filteredText.length < 2) { - newHour = `${text}0`; - newSelection = 1; - } else { - newSelection = 2; - } - } else if (selectionHour.start === 0 && selectionHour.end === 2) { - if (filteredText <= 1) { - newHour = `${filteredText}0`; - newSelection = 1; - } else if (filteredText > 12 && filteredText <= 24) { - newHour = String(filteredText - 12).padStart(2, '0'); - newSelection = 2; - setAmPmValue(CONST.TIME_PERIOD.PM); - } else { - newHour = String(Math.min(filteredText, 12)).padStart(2, '0'); - newSelection = 2; - } + } else if (filteredText.length <= 1 && filteredText < 2) { + newHour = `${filteredText}0`; + newSelection = 1; + } else if (filteredText > 12 && filteredText <= 24) { + newHour = String(filteredText - 12).padStart(2, '0'); + newSelection = 2; + setAmPmValue(CONST.TIME_PERIOD.PM); + } else if (filteredText.length <= 2) { + newHour = String(Math.min(filteredText, 12)).padStart(2, '0'); + newSelection = 2; } setHours(newHour); @@ -231,7 +221,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { if (selectionMinute.start === 0 && selectionMinute.end === 0) { // The cursor is at the start. const formattedText = `${filteredText[0]}${filteredText[2] || 0}`; - if (text[0] >= 6) { + if (filteredText[0] >= 6) { newMinute = `0${formattedText[1]}`; newSelection = 2; } else { @@ -242,27 +232,20 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { // The cursor is at the second position. // If we remove a value, prepend 0. if (filteredText.length < 2) { - newMinute = `0${text}`; + newMinute = `0${filteredText}`; newSelection = 0; setSelectionHour({start: 2, end: 2}); hourInputRef.current.focus(); } else { - newMinute = `${text[0]}${text[1]}`; + newMinute = `${filteredText[0]}${filteredText[1]}`; newSelection = 2; } - } else if (selectionMinute.start === 2 && selectionMinute.end === 2 && filteredText.length < 2) { - // The cursor is at the end and no text is selected - newMinute = `${text}0`; + } else if (filteredText.length <= 1 && filteredText <= 5) { + newMinute = `${filteredText}0`; newSelection = 1; - } else if (selectionMinute.start === 0 && selectionMinute.end === 2) { - // The user selects and replaces the text - if (filteredText <= 5) { - newMinute = `${filteredText}0`; - newSelection = 1; - } else { - newMinute = String(Math.min(filteredText, 59)).padStart(2, '0'); - newSelection = 2; - } + } else if (filteredText.length <= 2) { + newMinute = String(Math.min(filteredText, 59)).padStart(2, '0'); + newSelection = 2; } setMinute(newMinute); @@ -362,10 +345,11 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { if (selectionMinute.start !== 0 || selectionMinute.end !== 0 || e.key !== 'Backspace') { return; } - hourInputRef.current.focus(); + e.preventDefault(); + focusHourInputOnLastCharacter(); }, // eslint-disable-next-line react-hooks/exhaustive-deps - [selectionMinute.start, selectionMinute.end], + [selectionMinute.start, selectionMinute.end, focusHourInputOnLastCharacter], ); const {styleForAM, styleForPM} = StyleUtils.getStatusAMandPMButtonStyle(amPmValue); From 33a996a08e83aed8c1ddd08cab57bd30b82abb79 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Fri, 22 Dec 2023 09:54:50 +0100 Subject: [PATCH 04/12] Add explanatory comments --- src/components/TimePicker/TimePicker.js | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/components/TimePicker/TimePicker.js b/src/components/TimePicker/TimePicker.js index 2ae364de4c26..b0862fb08618 100644 --- a/src/components/TimePicker/TimePicker.js +++ b/src/components/TimePicker/TimePicker.js @@ -122,7 +122,6 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { const resetMinutes = () => { setMinute('00'); setSelectionMinute({start: 0, end: 0}); - focusHourInputOnLastCharacter(); }; // This function receive value from hour input and validate it @@ -145,6 +144,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { let newSelection = selectionHour.start; if (selectionHour.start === 0 && selectionHour.end === 0) { + // The cursor is at the start of hours // When the user is entering text, the filteredText consists of three numbers const formattedText = `${filteredText[0]}${filteredText[2] || 0}`; if (formattedText > 12 && formattedText <= 24) { @@ -160,7 +160,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newSelection = 1; } } else if (selectionHour.start === 1 && selectionHour.end === 1) { - // The cursor is at the second position. + // The cursor is in-between the digits const formattedText = `${filteredText[0]}${filteredText[1]}`; if (filteredText.length < 2) { @@ -181,13 +181,20 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newSelection = 2; } } else if (filteredText.length <= 1 && filteredText < 2) { + /* + The filtered text is either 0 or 1. We must check the length of the filtered text to avoid incorrectly handling e.g. "01" as "1" + We are either replacing hours with a single digit, or removing the last digit. + In both cases, we should append 0 to the remaining value. + */ newHour = `${filteredText}0`; newSelection = 1; } else if (filteredText > 12 && filteredText <= 24) { + // We are replacing hours with a value between 12 and 24. Switch AM to PM newHour = String(filteredText - 12).padStart(2, '0'); newSelection = 2; setAmPmValue(CONST.TIME_PERIOD.PM); } else if (filteredText.length <= 2) { + // We are replacing hours with a value either 2-11, or 24+. Minimize the value to 12 and prepend 0 if needed newHour = String(Math.min(filteredText, 12)).padStart(2, '0'); newSelection = 2; } @@ -219,7 +226,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { let newSelection = selectionMinute.start; if (selectionMinute.start === 0 && selectionMinute.end === 0) { - // The cursor is at the start. + // The cursor is at the start of minutes const formattedText = `${filteredText[0]}${filteredText[2] || 0}`; if (filteredText[0] >= 6) { newMinute = `0${formattedText[1]}`; @@ -229,9 +236,9 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newSelection = 1; } } else if (selectionMinute.start === 1 && selectionMinute.end === 1) { - // The cursor is at the second position. - // If we remove a value, prepend 0. + // The cursor is in-between the digits if (filteredText.length < 2) { + // If we remove a value, prepend 0. newMinute = `0${filteredText}`; newSelection = 0; setSelectionHour({start: 2, end: 2}); @@ -241,9 +248,15 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newSelection = 2; } } else if (filteredText.length <= 1 && filteredText <= 5) { + /* + The filtered text is from 0 to 5. We must check the length of the filtered text to avoid incorrectly handling e.g. "01" as "1" + We are either replacing minutes with a single digit, or removing the last digit. + In both cases, we should append 0 to the remaining value. + */ newMinute = `${filteredText}0`; newSelection = 1; } else if (filteredText.length <= 2) { + // We are replacing minutes with a value of 6+. Minimize the value to 59 and prepend 0 if needed newMinute = String(Math.min(filteredText, 59)).padStart(2, '0'); newSelection = 2; } From 1b3bcae10541859ac79af2e4ca3a8dd236283ae2 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Tue, 26 Dec 2023 22:18:38 +0100 Subject: [PATCH 05/12] Update the hours handling logic --- src/components/TimePicker/TimePicker.js | 85 ++++++++++++------------- 1 file changed, 41 insertions(+), 44 deletions(-) diff --git a/src/components/TimePicker/TimePicker.js b/src/components/TimePicker/TimePicker.js index b0862fb08618..50c7bdfae1ce 100644 --- a/src/components/TimePicker/TimePicker.js +++ b/src/components/TimePicker/TimePicker.js @@ -127,76 +127,73 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { // This function receive value from hour input and validate it // The valid format is HH(from 00 to 12). If the user input 9, it will be 09. If user try to change 09 to 19 it would skip the first character const handleHourChange = (text) => { - if (_.isEmpty(text)) { + const trimmedText = text.trim(); + + if (_.isEmpty(trimmedText)) { resetHours(); return; } - const isOnlyNumericValue = /^\d+$/.test(text.trim()); + const isOnlyNumericValue = /^\d+$/.test(trimmedText); if (!isOnlyNumericValue) { return; } - // Remove non-numeric characters. - const filteredText = text.replace(/[^0-9]/g, ''); - - let newHour = hours; - let newSelection = selectionHour.start; + let newHour; + let newSelection; if (selectionHour.start === 0 && selectionHour.end === 0) { // The cursor is at the start of hours - // When the user is entering text, the filteredText consists of three numbers - const formattedText = `${filteredText[0]}${filteredText[2] || 0}`; - if (formattedText > 12 && formattedText <= 24) { - // The hour is between 12 and 24 – switch AM to PM. - newHour = String(formattedText - 12).padStart(2, '0'); - newSelection = 2; - setAmPmValue(CONST.TIME_PERIOD.PM); - } else if (formattedText > 24) { - newHour = `0${formattedText[1]}`; - newSelection = 2; - } else { - newHour = `${formattedText[0]}${formattedText[1]}`; + const firstDigit = trimmedText[0]; + const secondDigit = trimmedText[2]; + + if (firstDigit <= 1) { + /* + The first entered digit is 0 or 1. + If the first digit is 0, we can safely append the second digit. + If the first digit is 1, we must check the second digit to ensure it is not greater than 2, amd replace it with 0 otherwise. + */ + newHour = `${firstDigit}${firstDigit === "1" && secondDigit > 1 ? 0 : secondDigit}`; newSelection = 1; + } else { + /* + The first entered digit is 2-9. + We should replace the whole value by prepending 0 to the entered digit. + */ + newHour = `0${firstDigit}`; + newSelection = 2; } } else if (selectionHour.start === 1 && selectionHour.end === 1) { // The cursor is in-between the digits - const formattedText = `${filteredText[0]}${filteredText[1]}`; - - if (filteredText.length < 2) { - // If we remove a value, prepend 0. - newHour = `0${filteredText}`; + if (trimmedText.length < 2) { + // We have removed the first digit. Replace it with 0 and move the cursor to the start. + newHour = `0${trimmedText}`; newSelection = 0; - // If the second digit is > 2, replace the hour with 0 and the second digit. - } else if (formattedText > 12 && formattedText <= 24) { - newHour = String(formattedText - 12).padStart(2, '0'); - newSelection = 2; - setAmPmValue(CONST.TIME_PERIOD.PM); - } else if (formattedText > 24) { - newHour = `0${filteredText[1]}`; - newSelection = 2; } else { - newHour = `${filteredText[0]}${filteredText[1]}`; - setHours(newHour); + newHour = `${trimmedText[0]}${trimmedText[1] || 0}`; newSelection = 2; } - } else if (filteredText.length <= 1 && filteredText < 2) { + } else if (trimmedText.length <= 1 && trimmedText <= 1) { /* - The filtered text is either 0 or 1. We must check the length of the filtered text to avoid incorrectly handling e.g. "01" as "1" + The trimmed text is either 0 or 1. We are either replacing hours with a single digit, or removing the last digit. In both cases, we should append 0 to the remaining value. + Note: we must check the length of the filtered text to avoid incorrectly handling e.g. "01" as "1". */ - newHour = `${filteredText}0`; + newHour = `${trimmedText}0`; newSelection = 1; - } else if (filteredText > 12 && filteredText <= 24) { - // We are replacing hours with a value between 12 and 24. Switch AM to PM - newHour = String(filteredText - 12).padStart(2, '0'); + } else { + newHour = trimmedText; newSelection = 2; + } + + if (newHour > 24) { + newHour = hours; + } else if (newHour > 12) { + newHour = String(newHour - 12).padStart(2, '0'); setAmPmValue(CONST.TIME_PERIOD.PM); - } else if (filteredText.length <= 2) { - // We are replacing hours with a value either 2-11, or 24+. Minimize the value to 12 and prepend 0 if needed - newHour = String(Math.min(filteredText, 12)).padStart(2, '0'); - newSelection = 2; + } else { + newHour = newHour.padStart(2, '0'); } setHours(newHour); From 37b65591871ebd6ae6ef233d86080db851e7b6bf Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Tue, 26 Dec 2023 23:01:35 +0100 Subject: [PATCH 06/12] Update the minutes handling logic --- src/components/TimePicker/TimePicker.js | 97 +++++++++++++------------ 1 file changed, 52 insertions(+), 45 deletions(-) diff --git a/src/components/TimePicker/TimePicker.js b/src/components/TimePicker/TimePicker.js index 50c7bdfae1ce..a9dd01e52d77 100644 --- a/src/components/TimePicker/TimePicker.js +++ b/src/components/TimePicker/TimePicker.js @@ -87,7 +87,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { const [selectionHour, setSelectionHour] = useState({start: 0, end: 0}); const [selectionMinute, setSelectionMinute] = useState({start: 2, end: 2}); // we focus it by default so need to have selection on the end const [hours, setHours] = useState(() => DateUtils.get12HourTimeObjectFromDate(value).hour); - const [minute, setMinute] = useState(() => DateUtils.get12HourTimeObjectFromDate(value).minute); + const [minutes, setMinutes] = useState(() => DateUtils.get12HourTimeObjectFromDate(value).minute); const [amPmValue, setAmPmValue] = useState(() => DateUtils.get12HourTimeObjectFromDate(value).period); const hourInputRef = useRef(null); @@ -107,11 +107,11 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { const validate = useCallback( (time) => { - const isValid = DateUtils.isTimeAtLeastOneMinuteInFuture({timeString: time || `${hours}:${minute} ${amPmValue}`, dateTimeString: defaultValue}); + const isValid = DateUtils.isTimeAtLeastOneMinuteInFuture({timeString: time || `${hours}:${minutes} ${amPmValue}`, dateTimeString: defaultValue}); setError(!isValid); return isValid; }, - [hours, minute, amPmValue, defaultValue], + [hours, minutes, amPmValue, defaultValue], ); const resetHours = () => { @@ -120,7 +120,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { }; const resetMinutes = () => { - setMinute('00'); + setMinutes('00'); setSelectionMinute({start: 0, end: 0}); }; @@ -153,13 +153,10 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { If the first digit is 0, we can safely append the second digit. If the first digit is 1, we must check the second digit to ensure it is not greater than 2, amd replace it with 0 otherwise. */ - newHour = `${firstDigit}${firstDigit === "1" && secondDigit > 1 ? 0 : secondDigit}`; + newHour = `${firstDigit}${firstDigit === '1' && secondDigit > 1 ? 0 : secondDigit}`; newSelection = 1; } else { - /* - The first entered digit is 2-9. - We should replace the whole value by prepending 0 to the entered digit. - */ + // The first entered digit is 2-9. We should replace the whole value by prepending 0 to the entered digit. newHour = `0${firstDigit}`; newSelection = 2; } @@ -203,63 +200,73 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } }; - // This function receive value from minute input and validate it - // The valid format is MM(from 00 to 59). If the user input 9, it will be 09. If user try to change 09 to 99 it would skip the character + /* + This function receives value from the minutes input and validates it. + The valid format is MM(from 00 to 59). If the user enters 9, it will be prepended to 09. If the user tries to change 09 to 99, it would skip the character + */ const handleMinutesChange = (text) => { - if (_.isEmpty(text)) { + const trimmedText = text.trim(); + + if (_.isEmpty(trimmedText)) { resetMinutes(); return; } - const isOnlyNumericValue = /^\d+$/.test(text.trim()); + const isOnlyNumericValue = /^\d+$/.test(trimmedText); if (!isOnlyNumericValue) { return; } - // Remove non-numeric characters. - const filteredText = text.replace(/[^0-9]/g, ''); - - let newMinute = minute; - let newSelection = selectionMinute.start; + let newMinute; + let newSelection; if (selectionMinute.start === 0 && selectionMinute.end === 0) { // The cursor is at the start of minutes - const formattedText = `${filteredText[0]}${filteredText[2] || 0}`; - if (filteredText[0] >= 6) { - newMinute = `0${formattedText[1]}`; - newSelection = 2; - } else { - newMinute = `${formattedText[0]}${formattedText[1]}`; + const firstDigit = trimmedText[0]; + if (firstDigit <= 5) { + // The first entered digit is 0-5, we can safely append the second digit. + newMinute = `${firstDigit}${trimmedText[2] || 0}`; newSelection = 1; + } else { + // The first entered digit is 6-9. We should replace the whole value by prepending 0 to the entered digit. + newMinute = `0${firstDigit}`; + newSelection = 2; } } else if (selectionMinute.start === 1 && selectionMinute.end === 1) { // The cursor is in-between the digits - if (filteredText.length < 2) { - // If we remove a value, prepend 0. - newMinute = `0${filteredText}`; + if (trimmedText.length < 2) { + // We have removed the first digit. Replace it with 0 and move the cursor to the start. + newMinute = `0${trimmedText}`; newSelection = 0; - setSelectionHour({start: 2, end: 2}); - hourInputRef.current.focus(); } else { - newMinute = `${filteredText[0]}${filteredText[1]}`; + newMinute = `${trimmedText[0]}${trimmedText[1] || 0}`; newSelection = 2; } - } else if (filteredText.length <= 1 && filteredText <= 5) { + } else if (trimmedText.length <= 1 && trimmedText <= 5) { /* - The filtered text is from 0 to 5. We must check the length of the filtered text to avoid incorrectly handling e.g. "01" as "1" + The trimmed text is from 0 to 5. We are either replacing minutes with a single digit, or removing the last digit. In both cases, we should append 0 to the remaining value. + Note: we must check the length of the filtered text to avoid incorrectly handling e.g. "01" as "1" */ - newMinute = `${filteredText}0`; + newMinute = `${trimmedText}0`; newSelection = 1; - } else if (filteredText.length <= 2) { - // We are replacing minutes with a value of 6+. Minimize the value to 59 and prepend 0 if needed - newMinute = String(Math.min(filteredText, 59)).padStart(2, '0'); + } else { + newMinute = trimmedText; newSelection = 2; } - setMinute(newMinute); + if (newMinute > 59) { + newMinute = minutes; + } else { + newMinute = newMinute.padStart(2, '0'); + } + + setMinutes(newMinute); setSelectionMinute({start: newSelection, end: newSelection}); + if (newSelection === 0) { + focusHourInputOnLastCharacter(); + } }; /** @@ -300,8 +307,8 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { return; } - const newMinute = replaceWithZeroAtPosition(minute, selectionMinute.start); - setMinute(newMinute); + const newMinute = replaceWithZeroAtPosition(minutes, selectionMinute.start); + setMinutes(newMinute); setSelectionMinute(decreaseBothSelectionByOne(selectionMinute)); } return; @@ -311,11 +318,11 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { if (isHourFocused) { handleHourChange(insertAtPosition(hours, trimmedKey, selectionHour.start, selectionHour.end)); } else if (isMinuteFocused) { - handleMinutesChange(insertAtPosition(minute, trimmedKey, selectionMinute.start, selectionMinute.end)); + handleMinutesChange(insertAtPosition(minutes, trimmedKey, selectionMinute.start, selectionMinute.end)); } }, // eslint-disable-next-line react-hooks/exhaustive-deps - [minute, hours, selectionHour, selectionMinute], + [minutes, hours, selectionHour, selectionMinute], ); useEffect(() => { @@ -378,12 +385,12 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { }, [canUseTouchScreen, updateAmountNumberPad]); useEffect(() => { - onInputChange(`${hours}:${minute} ${amPmValue}`); + onInputChange(`${hours}:${minutes} ${amPmValue}`); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [hours, minute, amPmValue]); + }, [hours, minutes, amPmValue]); const handleSubmit = () => { - const time = `${hours}:${minute} ${amPmValue}`; + const time = `${hours}:${minutes} ${amPmValue}`; const isValid = validate(time); if (isValid) { @@ -425,7 +432,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { Date: Thu, 28 Dec 2023 14:34:06 +0100 Subject: [PATCH 07/12] Handle digits removal by Backspace & Delete --- src/components/TimePicker/TimePicker.js | 46 +++++++++++++++---------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/src/components/TimePicker/TimePicker.js b/src/components/TimePicker/TimePicker.js index a9dd01e52d77..082bf4accaeb 100644 --- a/src/components/TimePicker/TimePicker.js +++ b/src/components/TimePicker/TimePicker.js @@ -16,7 +16,6 @@ import useWindowDimensions from '@hooks/useWindowDimensions'; import DateUtils from '@libs/DateUtils'; import * as DeviceCapabilities from '@libs/DeviceCapabilities'; import CONST from '@src/CONST'; -import setSelection from './setSelection'; const propTypes = { /** Refs forwarded to the TextInputWithCurrencySymbol */ @@ -89,13 +88,17 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { const [hours, setHours] = useState(() => DateUtils.get12HourTimeObjectFromDate(value).hour); const [minutes, setMinutes] = useState(() => DateUtils.get12HourTimeObjectFromDate(value).minute); const [amPmValue, setAmPmValue] = useState(() => DateUtils.get12HourTimeObjectFromDate(value).period); + const [lastPressedKey, setLastPressedKey] = useState(''); const hourInputRef = useRef(null); const minuteInputRef = useRef(null); const focusMinuteInputOnFirstCharacter = useCallback(() => { - const cleanupTimer = setSelection({start: 0, end: 0}, minuteInputRef, setSelectionMinute); - return cleanupTimer; + setSelectionMinute({start: 0, end: 0}); + const timer = setTimeout(() => { + minuteInputRef.current.focus(); + }, 10); + return () => clearTimeout(timer); }, []); const focusHourInputOnLastCharacter = useCallback(() => { setSelectionHour({start: 2, end: 2}); @@ -145,15 +148,18 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { if (selectionHour.start === 0 && selectionHour.end === 0) { // The cursor is at the start of hours const firstDigit = trimmedText[0]; - const secondDigit = trimmedText[2]; + const secondDigit = trimmedText[2] || '0'; - if (firstDigit <= 1) { + if (trimmedText.length === 1) { + newHour = `0${firstDigit}`; + newSelection = 1; + } else if (firstDigit <= 1) { /* The first entered digit is 0 or 1. If the first digit is 0, we can safely append the second digit. If the first digit is 1, we must check the second digit to ensure it is not greater than 2, amd replace it with 0 otherwise. */ - newHour = `${firstDigit}${firstDigit === '1' && secondDigit > 1 ? 0 : secondDigit}`; + newHour = `${firstDigit}${firstDigit === '1' && secondDigit > 2 ? 0 : secondDigit}`; newSelection = 1; } else { // The first entered digit is 2-9. We should replace the whole value by prepending 0 to the entered digit. @@ -162,7 +168,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } } else if (selectionHour.start === 1 && selectionHour.end === 1) { // The cursor is in-between the digits - if (trimmedText.length < 2) { + if (trimmedText.length < 2 && lastPressedKey === 'Backspace') { // We have removed the first digit. Replace it with 0 and move the cursor to the start. newHour = `0${trimmedText}`; newSelection = 0; @@ -180,7 +186,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newHour = `${trimmedText}0`; newSelection = 1; } else { - newHour = trimmedText; + newHour = trimmedText.substring(0, 2).padStart(2, '0'); newSelection = 2; } @@ -189,8 +195,6 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } else if (newHour > 12) { newHour = String(newHour - 12).padStart(2, '0'); setAmPmValue(CONST.TIME_PERIOD.PM); - } else { - newHour = newHour.padStart(2, '0'); } setHours(newHour); @@ -223,7 +227,10 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { if (selectionMinute.start === 0 && selectionMinute.end === 0) { // The cursor is at the start of minutes const firstDigit = trimmedText[0]; - if (firstDigit <= 5) { + if (trimmedText.length === 1) { + newMinute = `0${firstDigit}`; + newSelection = 1; + } else if (firstDigit <= 5) { // The first entered digit is 0-5, we can safely append the second digit. newMinute = `${firstDigit}${trimmedText[2] || 0}`; newSelection = 1; @@ -234,7 +241,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } } else if (selectionMinute.start === 1 && selectionMinute.end === 1) { // The cursor is in-between the digits - if (trimmedText.length < 2) { + if (trimmedText.length < 2 && lastPressedKey === 'Backspace') { // We have removed the first digit. Replace it with 0 and move the cursor to the start. newMinute = `0${trimmedText}`; newSelection = 0; @@ -252,21 +259,16 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newMinute = `${trimmedText}0`; newSelection = 1; } else { - newMinute = trimmedText; + newMinute = trimmedText.substring(0, 2).padStart(2, '0'); newSelection = 2; } if (newMinute > 59) { newMinute = minutes; - } else { - newMinute = newMinute.padStart(2, '0'); } setMinutes(newMinute); setSelectionMinute({start: newSelection, end: newSelection}); - if (newSelection === 0) { - focusHourInputOnLastCharacter(); - } }; /** @@ -409,6 +411,9 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { placeholder={numberFormat(0)} maxLength={2} formattedAmount={hours} + onKeyPress={(e) => { + setLastPressedKey(e.nativeEvent.key); + }} onChangeAmount={handleHourChange} role={CONST.ACCESSIBILITY_ROLE.TEXT} ref={(ref) => { @@ -433,7 +438,10 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { placeholder={numberFormat(0)} maxLength={2} formattedAmount={minutes} - onKeyPress={handleFocusOnBackspace} + onKeyPress={(e) => { + setLastPressedKey(e.nativeEvent.key); + handleFocusOnBackspace(e); + }} onChangeAmount={handleMinutesChange} role={CONST.ACCESSIBILITY_ROLE.TEXT} ref={(ref) => { From 367f9f1dd5f77f1070f25495eb205fc3e8ca96aa Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Thu, 28 Dec 2023 20:39:05 +0100 Subject: [PATCH 08/12] Squash setSelection into one reusable function without using timeouts --- src/components/TimePicker/TimePicker.js | 32 +++++++++---------- src/components/TimePicker/setSelection.ios.ts | 8 ----- src/components/TimePicker/setSelection.ts | 18 ----------- 3 files changed, 16 insertions(+), 42 deletions(-) delete mode 100644 src/components/TimePicker/setSelection.ios.ts delete mode 100644 src/components/TimePicker/setSelection.ts diff --git a/src/components/TimePicker/TimePicker.js b/src/components/TimePicker/TimePicker.js index 082bf4accaeb..ced018b56b7d 100644 --- a/src/components/TimePicker/TimePicker.js +++ b/src/components/TimePicker/TimePicker.js @@ -74,6 +74,16 @@ function replaceWithZeroAtPosition(originalString, position) { return `${originalString.slice(0, position - 1)}0${originalString.slice(position)}`; } +function setCursorPosition(position, ref, setSelection) { + const selection = { + start: position, + end: position, + }; + setSelection(selection); + ref.current?.setNativeProps({selection}); + ref.current?.focus(); +} + function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { const {numberFormat, translate} = useLocalize(); const {isExtraSmallScreenHeight} = useWindowDimensions(); @@ -93,20 +103,8 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { const hourInputRef = useRef(null); const minuteInputRef = useRef(null); - const focusMinuteInputOnFirstCharacter = useCallback(() => { - setSelectionMinute({start: 0, end: 0}); - const timer = setTimeout(() => { - minuteInputRef.current.focus(); - }, 10); - return () => clearTimeout(timer); - }, []); - const focusHourInputOnLastCharacter = useCallback(() => { - setSelectionHour({start: 2, end: 2}); - const timer = setTimeout(() => { - hourInputRef.current.focus(); - }, 10); - return () => clearTimeout(timer); - }, []); + const focusMinuteInputOnFirstCharacter = useCallback(() => setCursorPosition(0, minuteInputRef, setSelectionMinute), []); + const focusHourInputOnLastCharacter = useCallback(() => setCursorPosition(2, hourInputRef, setSelectionHour), []); const validate = useCallback( (time) => { @@ -340,17 +338,19 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { [], ); - const arrowLeftCallback = useCallback(() => { + const arrowLeftCallback = useCallback((e) => { const isMinuteFocused = minuteInputRef.current.isFocused(); if (isMinuteFocused && selectionMinute.start === 0) { + e.preventDefault(); focusHourInputOnLastCharacter(); } // eslint-disable-next-line react-hooks/exhaustive-deps }, [selectionHour, selectionMinute]); - const arrowRightCallback = useCallback(() => { + const arrowRightCallback = useCallback((e) => { const isHourFocused = hourInputRef.current.isFocused(); if (isHourFocused && selectionHour.start === 2) { + e.preventDefault(); focusMinuteInputOnFirstCharacter(); } // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/src/components/TimePicker/setSelection.ios.ts b/src/components/TimePicker/setSelection.ios.ts deleted file mode 100644 index 0d1dfc004bc7..000000000000 --- a/src/components/TimePicker/setSelection.ios.ts +++ /dev/null @@ -1,8 +0,0 @@ -import {RefObject} from 'react'; -import {TextInput} from 'react-native'; - -export default function setSelection(value: {start: number; end: number}, ref: RefObject) { - ref.current?.focus(); - ref.current?.setNativeProps({selection: value}); - return () => {}; -} diff --git a/src/components/TimePicker/setSelection.ts b/src/components/TimePicker/setSelection.ts deleted file mode 100644 index 36304b408f29..000000000000 --- a/src/components/TimePicker/setSelection.ts +++ /dev/null @@ -1,18 +0,0 @@ -// setSelection.ts -import {RefObject} from 'react'; -import {TextInput} from 'react-native'; - -const setSelection = (value: {start: number; end: number}, ref: RefObject, setSelectionCallback: (value: {start: number; end: number}) => void): (() => void) => { - ref.current?.focus(); - - const timer = setTimeout(() => { - setSelectionCallback(value); - }, 10); - - // Return the cleanup function - return () => { - clearTimeout(timer); - }; -}; - -export default setSelection; From be0342f71f6e15b6ba4e3709a3daa948eee20f02 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Thu, 28 Dec 2023 23:40:08 +0100 Subject: [PATCH 09/12] Revert the setSelection to be ios-specific --- src/components/TimePicker/TimePicker.js | 69 ++++++++++--------- .../TimePicker/setCursorPosition/index.ios.ts | 13 ++++ .../TimePicker/setCursorPosition/index.ts | 11 +++ .../TimePicker/setCursorPosition/types.ts | 6 ++ 4 files changed, 66 insertions(+), 33 deletions(-) create mode 100644 src/components/TimePicker/setCursorPosition/index.ios.ts create mode 100644 src/components/TimePicker/setCursorPosition/index.ts create mode 100644 src/components/TimePicker/setCursorPosition/types.ts diff --git a/src/components/TimePicker/TimePicker.js b/src/components/TimePicker/TimePicker.js index ced018b56b7d..827d8851d6f1 100644 --- a/src/components/TimePicker/TimePicker.js +++ b/src/components/TimePicker/TimePicker.js @@ -16,6 +16,7 @@ import useWindowDimensions from '@hooks/useWindowDimensions'; import DateUtils from '@libs/DateUtils'; import * as DeviceCapabilities from '@libs/DeviceCapabilities'; import CONST from '@src/CONST'; +import setCursorPosition from './setCursorPosition'; const propTypes = { /** Refs forwarded to the TextInputWithCurrencySymbol */ @@ -59,7 +60,7 @@ function insertAtPosition(originalString, newSubstring, selectionPositionFrom, s return originalString.slice(0, selectionPositionFrom) + newSubstring + originalString.slice(selectionPositionTo); } -// if we need manually to move selection to the left we need to decrease both selection start and end by one +// if we need to manually move selection to the left, we need to decrease both selection start and end by one function decreaseBothSelectionByOne({start, end}) { if (start === 0) { return {start: 0, end: 0}; @@ -74,16 +75,6 @@ function replaceWithZeroAtPosition(originalString, position) { return `${originalString.slice(0, position - 1)}0${originalString.slice(position)}`; } -function setCursorPosition(position, ref, setSelection) { - const selection = { - start: position, - end: position, - }; - setSelection(selection); - ref.current?.setNativeProps({selection}); - ref.current?.focus(); -} - function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { const {numberFormat, translate} = useLocalize(); const {isExtraSmallScreenHeight} = useWindowDimensions(); @@ -98,8 +89,8 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { const [hours, setHours] = useState(() => DateUtils.get12HourTimeObjectFromDate(value).hour); const [minutes, setMinutes] = useState(() => DateUtils.get12HourTimeObjectFromDate(value).minute); const [amPmValue, setAmPmValue] = useState(() => DateUtils.get12HourTimeObjectFromDate(value).period); - const [lastPressedKey, setLastPressedKey] = useState(''); + const lastPressedKey = useRef(''); const hourInputRef = useRef(null); const minuteInputRef = useRef(null); @@ -166,7 +157,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } } else if (selectionHour.start === 1 && selectionHour.end === 1) { // The cursor is in-between the digits - if (trimmedText.length < 2 && lastPressedKey === 'Backspace') { + if (trimmedText.length < 2 && lastPressedKey.current === 'Backspace') { // We have removed the first digit. Replace it with 0 and move the cursor to the start. newHour = `0${trimmedText}`; newSelection = 0; @@ -239,7 +230,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } } else if (selectionMinute.start === 1 && selectionMinute.end === 1) { // The cursor is in-between the digits - if (trimmedText.length < 2 && lastPressedKey === 'Backspace') { + if (trimmedText.length < 2 && lastPressedKey.current === 'Backspace') { // We have removed the first digit. Replace it with 0 and move the cursor to the start. newMinute = `0${trimmedText}`; newSelection = 0; @@ -338,23 +329,35 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { [], ); - const arrowLeftCallback = useCallback((e) => { - const isMinuteFocused = minuteInputRef.current.isFocused(); - if (isMinuteFocused && selectionMinute.start === 0) { - e.preventDefault(); - focusHourInputOnLastCharacter(); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [selectionHour, selectionMinute]); - const arrowRightCallback = useCallback((e) => { - const isHourFocused = hourInputRef.current.isFocused(); + const arrowLeftCallback = useCallback( + (e) => { + const isMinuteFocused = minuteInputRef.current.isFocused(); + if (isMinuteFocused && selectionMinute.start === 0) { + if (e) { + // Check e to be truthy to avoid crashing on Android (e is undefined there) + e.preventDefault(); + } + focusHourInputOnLastCharacter(); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, + [selectionHour, selectionMinute], + ); + const arrowRightCallback = useCallback( + (e) => { + const isHourFocused = hourInputRef.current.isFocused(); - if (isHourFocused && selectionHour.start === 2) { - e.preventDefault(); - focusMinuteInputOnFirstCharacter(); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [selectionHour, selectionMinute]); + if (isHourFocused && selectionHour.start === 2) { + if (e) { + // Check e to be truthy to avoid crashing on Android (e is undefined there) + e.preventDefault(); + } + focusMinuteInputOnFirstCharacter(); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, + [selectionHour, selectionMinute], + ); useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_LEFT, arrowLeftCallback, arrowConfig); useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_RIGHT, arrowRightCallback, arrowConfig); @@ -381,7 +384,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { ); }, [canUseTouchScreen, updateAmountNumberPad]); @@ -412,7 +415,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { maxLength={2} formattedAmount={hours} onKeyPress={(e) => { - setLastPressedKey(e.nativeEvent.key); + lastPressedKey.current = e.nativeEvent.key; }} onChangeAmount={handleHourChange} role={CONST.ACCESSIBILITY_ROLE.TEXT} @@ -439,7 +442,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { maxLength={2} formattedAmount={minutes} onKeyPress={(e) => { - setLastPressedKey(e.nativeEvent.key); + lastPressedKey.current = e.nativeEvent.key; handleFocusOnBackspace(e); }} onChangeAmount={handleMinutesChange} diff --git a/src/components/TimePicker/setCursorPosition/index.ios.ts b/src/components/TimePicker/setCursorPosition/index.ios.ts new file mode 100644 index 000000000000..7e51abc3212e --- /dev/null +++ b/src/components/TimePicker/setCursorPosition/index.ios.ts @@ -0,0 +1,13 @@ +import SetCursorPosition from './types'; + +const setCursorPosition: SetCursorPosition = (position, ref, setSelection) => { + const selection = { + start: position, + end: position, + }; + setSelection(selection); + ref.current?.focus(); + ref.current?.setNativeProps({selection}); +}; + +export default setCursorPosition; diff --git a/src/components/TimePicker/setCursorPosition/index.ts b/src/components/TimePicker/setCursorPosition/index.ts new file mode 100644 index 000000000000..4d114641909f --- /dev/null +++ b/src/components/TimePicker/setCursorPosition/index.ts @@ -0,0 +1,11 @@ +import SetCursorPosition from './types'; + +const setCursorPosition: SetCursorPosition = (position, ref, setSelection) => { + setSelection({ + start: position, + end: position, + }); + ref.current?.focus(); +}; + +export default setCursorPosition; diff --git a/src/components/TimePicker/setCursorPosition/types.ts b/src/components/TimePicker/setCursorPosition/types.ts new file mode 100644 index 000000000000..f7fd20a59e8e --- /dev/null +++ b/src/components/TimePicker/setCursorPosition/types.ts @@ -0,0 +1,6 @@ +import {RefObject} from 'react'; +import {TextInput} from 'react-native'; + +type SetCursorPosition = (position: number, ref: RefObject, setSelection: (value: {start: number; end: number}) => void) => void; + +export default SetCursorPosition; From aeffff1fe5bbf1456c4d2fa7e328526f30b136e9 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Fri, 29 Dec 2023 11:14:51 +0100 Subject: [PATCH 10/12] Handle single-digit selection removal using NumberPad --- src/components/TimePicker/TimePicker.js | 53 +++++++++++-------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/src/components/TimePicker/TimePicker.js b/src/components/TimePicker/TimePicker.js index 827d8851d6f1..b22efb489100 100644 --- a/src/components/TimePicker/TimePicker.js +++ b/src/components/TimePicker/TimePicker.js @@ -60,19 +60,28 @@ function insertAtPosition(originalString, newSubstring, selectionPositionFrom, s return originalString.slice(0, selectionPositionFrom) + newSubstring + originalString.slice(selectionPositionTo); } -// if we need to manually move selection to the left, we need to decrease both selection start and end by one -function decreaseBothSelectionByOne({start, end}) { - if (start === 0) { - return {start: 0, end: 0}; - } - return {start: start - 1, end: end - 1}; +function replaceRangeWithZeros(originalString, from, to) { + const normalizedFrom = Math.max(from, 0); + const normalizedTo = Math.min(to, 2); + const replacement = '0'.repeat(normalizedTo - normalizedFrom); + return `${originalString.slice(0, normalizedFrom)}${replacement}${originalString.slice(normalizedTo)}`; } -function replaceWithZeroAtPosition(originalString, position) { - if (position === 0 || position > 2) { - return originalString; +function clearSelectedValue(value, selection, setValue, setSelection) { + let newValue; + let newCursorPosition; + + if (selection.start !== selection.end) { + newValue = replaceRangeWithZeros(value, selection.start, selection.end); + newCursorPosition = selection.start; + } else { + const positionBeforeSelection = Math.max(selection.start - 1, 0); + newValue = replaceRangeWithZeros(value, positionBeforeSelection, selection.start); + newCursorPosition = positionBeforeSelection; } - return `${originalString.slice(0, position - 1)}0${originalString.slice(position)}`; + + setValue(newValue); + setSelection({start: newCursorPosition, end: newCursorPosition}); } function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { @@ -279,28 +288,14 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } if (key === '<' || key === 'Backspace') { if (isHourFocused) { - if (selectionHour.start === 0 && selectionHour.end === 2) { - resetHours(); - return; - } - - const newHour = replaceWithZeroAtPosition(hours, selectionHour.start); - setHours(newHour); - setSelectionHour(decreaseBothSelectionByOne(selectionHour)); + clearSelectedValue(hours, selectionHour, setHours, setSelectionHour); } else if (isMinuteFocused) { - if (selectionMinute.start === 0 && selectionMinute.end === 2) { - resetMinutes(); - return; - } - if (selectionMinute.start === 0 && selectionMinute.end === 0) { focusHourInputOnLastCharacter(); return; } - const newMinute = replaceWithZeroAtPosition(minutes, selectionMinute.start); - setMinutes(newMinute); - setSelectionMinute(decreaseBothSelectionByOne(selectionMinute)); + clearSelectedValue(minutes, selectionMinute, setMinutes, setSelectionMinute); } return; } @@ -313,7 +308,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } }, // eslint-disable-next-line react-hooks/exhaustive-deps - [minutes, hours, selectionHour, selectionMinute], + [minutes, hours, selectionMinute, selectionHour], ); useEffect(() => { @@ -339,8 +334,8 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } focusHourInputOnLastCharacter(); } - // eslint-disable-next-line react-hooks/exhaustive-deps }, + // eslint-disable-next-line react-hooks/exhaustive-deps [selectionHour, selectionMinute], ); const arrowRightCallback = useCallback( @@ -354,8 +349,8 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } focusMinuteInputOnFirstCharacter(); } - // eslint-disable-next-line react-hooks/exhaustive-deps }, + // eslint-disable-next-line react-hooks/exhaustive-deps [selectionHour, selectionMinute], ); From c46769bf95fe82162656c53595803e4112747234 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Fri, 29 Dec 2023 16:54:36 +0100 Subject: [PATCH 11/12] Support single-digit removal & replacement --- src/components/TimePicker/TimePicker.js | 34 +++++++++++++++++++------ 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/components/TimePicker/TimePicker.js b/src/components/TimePicker/TimePicker.js index b22efb489100..ea41d45956c8 100644 --- a/src/components/TimePicker/TimePicker.js +++ b/src/components/TimePicker/TimePicker.js @@ -128,8 +128,8 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { // This function receive value from hour input and validate it // The valid format is HH(from 00 to 12). If the user input 9, it will be 09. If user try to change 09 to 19 it would skip the first character const handleHourChange = (text) => { - const trimmedText = text.trim(); - + // Replace spaces with 0 to implement the following digit removal by pressing space + const trimmedText = text.replace(/ /g, '0'); if (_.isEmpty(trimmedText)) { resetHours(); return; @@ -149,6 +149,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { const secondDigit = trimmedText[2] || '0'; if (trimmedText.length === 1) { + // To support the forward-removal using Delete key newHour = `0${firstDigit}`; newSelection = 1; } else if (firstDigit <= 1) { @@ -166,7 +167,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } } else if (selectionHour.start === 1 && selectionHour.end === 1) { // The cursor is in-between the digits - if (trimmedText.length < 2 && lastPressedKey.current === 'Backspace') { + if (trimmedText.length === 1 && lastPressedKey.current === 'Backspace') { // We have removed the first digit. Replace it with 0 and move the cursor to the start. newHour = `0${trimmedText}`; newSelection = 0; @@ -174,7 +175,15 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newHour = `${trimmedText[0]}${trimmedText[1] || 0}`; newSelection = 2; } - } else if (trimmedText.length <= 1 && trimmedText <= 1) { + } else if (selectionHour.start === 0 && selectionHour.end === 1) { + // There is an active selection of the first digit + newHour = trimmedText.substring(0, 2).padStart(2, '0'); + newSelection = trimmedText.length === 1 ? 0 : 1; + } else if (selectionHour.start === 1 && selectionHour.end === 2) { + // There is an active selection of the second digit + newHour = trimmedText.substring(0, 2).padEnd(2, '0'); + newSelection = trimmedText.length === 1 ? 1 : 2; + } else if (trimmedText.length === 1 && trimmedText <= 1) { /* The trimmed text is either 0 or 1. We are either replacing hours with a single digit, or removing the last digit. @@ -207,8 +216,8 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { The valid format is MM(from 00 to 59). If the user enters 9, it will be prepended to 09. If the user tries to change 09 to 99, it would skip the character */ const handleMinutesChange = (text) => { - const trimmedText = text.trim(); - + // Replace spaces with 0 to implement the following digit removal by pressing space + const trimmedText = text.replace(/ /g, '0'); if (_.isEmpty(trimmedText)) { resetMinutes(); return; @@ -226,6 +235,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { // The cursor is at the start of minutes const firstDigit = trimmedText[0]; if (trimmedText.length === 1) { + // To support the forward-removal using Delete key newMinute = `0${firstDigit}`; newSelection = 1; } else if (firstDigit <= 5) { @@ -239,7 +249,7 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { } } else if (selectionMinute.start === 1 && selectionMinute.end === 1) { // The cursor is in-between the digits - if (trimmedText.length < 2 && lastPressedKey.current === 'Backspace') { + if (trimmedText.length === 1 && lastPressedKey.current === 'Backspace') { // We have removed the first digit. Replace it with 0 and move the cursor to the start. newMinute = `0${trimmedText}`; newSelection = 0; @@ -247,7 +257,15 @@ function TimePicker({forwardedRef, defaultValue, onSubmit, onInputChange}) { newMinute = `${trimmedText[0]}${trimmedText[1] || 0}`; newSelection = 2; } - } else if (trimmedText.length <= 1 && trimmedText <= 5) { + } else if (selectionMinute.start === 0 && selectionMinute.end === 1) { + // There is an active selection of the first digit + newMinute = trimmedText.substring(0, 2).padStart(2, '0'); + newSelection = trimmedText.length === 1 ? 0 : 1; + } else if (selectionMinute.start === 1 && selectionMinute.end === 2) { + // There is an active selection of the second digit + newMinute = trimmedText.substring(0, 2).padEnd(2, '0'); + newSelection = trimmedText.length === 1 ? 1 : 2; + } else if (trimmedText.length === 1 && trimmedText <= 5) { /* The trimmed text is from 0 to 5. We are either replacing minutes with a single digit, or removing the last digit. From e4bffbffa3ed95afc11433858a32078445bd82fa Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Tue, 2 Jan 2024 12:24:11 +0100 Subject: [PATCH 12/12] Add JSDocs --- src/components/TimePicker/TimePicker.js | 44 +++++++++++++++++++------ 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/src/components/TimePicker/TimePicker.js b/src/components/TimePicker/TimePicker.js index ea41d45956c8..20f52e2d08ee 100644 --- a/src/components/TimePicker/TimePicker.js +++ b/src/components/TimePicker/TimePicker.js @@ -42,24 +42,41 @@ const AMOUNT_VIEW_ID = 'amountView'; const NUM_PAD_CONTAINER_VIEW_ID = 'numPadContainerView'; const NUM_PAD_VIEW_ID = 'numPadView'; -function insertAtPosition(originalString, newSubstring, selectionPositionFrom, selectionPositionTo) { +/** + * Replace the sub-string of the given string with the provided value + * @param {String} originalString - the string that will be modified + * @param {String} newSubstring - the replacement string + * @param {Number} from - the start index of the sub-string to replace + * @param {Number} to - the end index of the sub-string to replace + * + * @returns {String} - the modified string with the range (from, to) replaced with the provided value + */ +function insertAtPosition(originalString, newSubstring, from, to) { // Check for invalid positions - if (selectionPositionFrom < 0 || selectionPositionTo < 0 || selectionPositionFrom > originalString.length || selectionPositionTo > originalString.length) { - return; + if (from < 0 || to < 0 || from > originalString.length || to > originalString.length) { + return originalString; } - // If the positions are the same, it means we're inserting at a point - if (selectionPositionFrom === selectionPositionTo) { - if (selectionPositionFrom === originalString.length) { - return originalString; // If the insertion point is at the end, simply return the original string - } - return originalString.slice(0, selectionPositionFrom) + newSubstring + originalString.slice(selectionPositionFrom); + /* + If the positions are the same, it means we're inserting at a point. + If the insertion point is at the end, simply return the original string. + */ + if (from === to && from === originalString.length) { + return originalString; } // Replace the selected range - return originalString.slice(0, selectionPositionFrom) + newSubstring + originalString.slice(selectionPositionTo); + return originalString.slice(0, from) + newSubstring + originalString.slice(to); } +/** + * Replace the sub-string of the given string with zeros + * @param {String} originalString - the string that will be modified + * @param {Number} from - the start index of the sub-string to replace + * @param {Number} to - the end index of the sub-string to replace + * + * @returns {String} - the modified string with the range (from, to) replaced with zeros + */ function replaceRangeWithZeros(originalString, from, to) { const normalizedFrom = Math.max(from, 0); const normalizedTo = Math.min(to, 2); @@ -67,6 +84,13 @@ function replaceRangeWithZeros(originalString, from, to) { return `${originalString.slice(0, normalizedFrom)}${replacement}${originalString.slice(normalizedTo)}`; } +/** + * Clear the value under selection of an input (either hours or minutes) by replacing it with zeros + * @param {String} value - current value of the input + * @param {Object} selection - current selection of the input + * @param {Function} setValue - the function that modifies the value of the input + * @param {Function} setSelection - the function that modifies the selection of the input + */ function clearSelectedValue(value, selection, setValue, setSelection) { let newValue; let newCursorPosition;