Skip to content

Commit

Permalink
[MIRROR] Makes the restricted number input user friendlier (#2043)
Browse files Browse the repository at this point in the history
* Makes the restricted number input user friendlier (#81495)

## About The Pull Request
The modal number input is extremely user unfriendly in it's current
state whenever used for negative and decimal numbers See #81457 for
details. The code might not be the best and if someone has suggestions
to optimize it, I'm all open for it, but it takes into aspect a lot of
edge cases to make the input as user friendly as possible. I've also
heard form some other code bases that they have issues with the
tgui_input_number proc.

The RestrictedNumber will get a new event to listen to though:

` onBlur={(_, value) => onBlur(value)}`

This will run the number through the previous, hard sanitation and
ensures that we always submit an allowed number and not a string value
as we are using it shortly during input for the pure `-` input.
(Especially important for the "Submit" button. Enter always handles it
that way). During testing, all the tested numbers were handled properly,
I could not see any issues on the inputs. We still keep some in Field
replacement, but it's for a limited range where we can have the field
handle numbers smart:

`min <= 1 && max >= 0`: Within that range, the field updates the value
to the restrains on input. In all other cases only onBlur.

Testcase table:


<html xmlns:v="urn:schemas-microsoft-com:vml"
xmlns:o="urn:schemas-microsoft-com:office:office"
xmlns:x="urn:schemas-microsoft-com:office:excel"
xmlns="http://www.w3.org/TR/REC-html40">

<head>

<meta name=ProgId content=Excel.Sheet>
<!--table
	{mso-displayed-decimal-separator:"\,";
	mso-displayed-thousand-separator:"\.";}
@page
	{margin:.79in .7in .79in .7in;
	mso-header-margin:.3in;
	mso-footer-margin:.3in;}
tr
	{mso-height-source:auto;}
col
	{mso-width-source:auto;}
br
	{mso-data-placement:same-cell;}
td
	{padding-top:1px;
	padding-right:1px;
	padding-left:1px;
	mso-ignore:padding;
	color:black;
	font-size:11.0pt;
	font-weight:400;
	font-style:normal;
	text-decoration:none;
	font-family:Calibri, sans-serif;
	mso-font-charset:0;
	mso-number-format:General;
	text-align:general;
	vertical-align:bottom;
	border:none;
	mso-background-source:auto;
	mso-pattern:auto;
	mso-protection:locked visible;
	white-space:nowrap;
	mso-rotate:0;}
.xl63
	{text-align:left;}
.xl64
	{mso-number-format:"dd\/\\ mmm";
	text-align:left;}
.xl65
	{text-align:left;
	vertical-align:top;}
.xl66
	{mso-number-format:"\#\,\#\#0";
	text-align:left;}
.xl67
	{mso-number-format:"\@";
	text-align:left;}
.xl68
	{mso-number-format:"mmm\\ yy";
	text-align:left;}
-->
</head>

<body link="#0563C1" vlink="#954F72">


Test case | Field Limit | Input | InField | OnEnter | OnSubmit | OnBlur
-- | -- | -- | -- | -- | -- | --
Float (smart) | 1-20 | 0 | 1 | 1 | 1 | 1
  |   | 1.1 | 1.1 | 1.1 | 1.1 | 1.1
  |   | 20.1 | 20 | 20 | 20 | 20
  |   | - |   | 1 | 1 | 1
  |   | . | 1. | 1. | 1. | 1.
  |   | 5 | 5 | 5 | 5 | 5
Float | 2-20 | 0 | 0 | 2 | 2 | 2
  |   | 2.1 | 2.1 | 2.1 | 2.1 | 2.1
  |   | 5 | 5 | 5 | 5 | 5
  |   | 20.1 | 20.1 | 20 | 20 | 20
  |   | - |   | 2 | 2 | 2
  |   | . | 2. | 2. | 2. | 2.
Float with decimal limit | 20.2-200.2 | 0 | 0 | 20.2 | 20.2 | 20.2
  |   | 20. | 20. | 20.2 | 20.2 | 20.2
  |   | 50 | 50 | 50 | 50 | 50
  |   | 200.3 | 200.3 | 200.2 | 200.2 | 200.2
  |   | - |   | 20.2 | 20.2 | 20.2
  |   | . | 20. | 20.2 | 20.2 | 20.2
Float with decimal limit   (smart) | 0-20.2 | 0 | 0 | 0 | 0 | 0
  |   | 20. | 20. | 20. | 20. | 20.
  |   | 5 | 5 | 5 | 5 | 5
  |   | 20.3 | 20.2 | 20.2 | 20.2 | 20.2
  |   | - |   | 0 | 0 | 0
  |   | . | 0. | 0. | 0. | 0.
Negative Float (smart) | -10-20 | -10.2 | -10 | -10 | -10 | -10
  |   | 20. | 20. | 20. | 20. | 20.
  |   | 20.1 | 20 | 20 | 20 | 20
  |   | 0 | 0 | 0 | 0 | 0
  |   | - | - | -10 | -10 | -10
  |   | . | -10 | -10 | -10 | -10
  |   | -. | -10. | -10. | -10. | -10.
  |   | -10- | 10 | 10 | 10 | 10
  |   | 10- | -10 | -10 | -10 | -10
Negative Float with decimal limit | -10.2--20.2 | -10.1 | -10.1 | -10.2
| -10.2 | -10.2
  |   | -20. | -20. | -20. | -20. | -20.
  |   | -20.3 | -20.3 | -20.2 | -20.2 | -20.2
  |   | 0 | 0 | -10.2 | -10.2 | -10.2
  |   | - | - | -20.2 | -20.2 | -20.2
  |   | . | -20. | -20. | -20. | -20.
  |   | -. | -20. | -20. | -20. | -20.
  |   | -10.2- | 10.2 | -10.2 | -10.2 | -10.2
  |   | 10.2- | -10.2 | -10.2 | -10.2 | -10.2
Int (smart) | 1-20 | 0 | 1 | 1 | 1 | 1
  |   | 1. | 1 | 1 | 1 | 1
  |   | 21 | 20 | 20 | 20 | 20
  |   | 5 | 5 | 5 | 5 | 5
  |   | - |   | 1 | 1 | 1
  |   | . |   | 1 | 1 | 1
Int | 20-200 | 19 | 19 | 20 | 20 | 20
  |   | 201 | 201 | 200 | 200 | 200
  |   | 50 | 50 | 50 | 50 | 50
  |   | - |   | 20 | 20 | 20
  |   | . |   | 20 | 20 | 20
Negative Int (smart) | -10-20 | -11 | -10 | -10 | -10 | -10
  |   | 0 | 0 | 0 | 0 | 0
  |   | 21 | 20 | 20 | 20 | 20
  |   | - | - | -10 | -10 | -10
  |   | . |   | -10 | -10 | -10
  |   | -. | - | -10 | -10 | -10
  |   | -10- | 10 | 10 | 10 | 10
  |   | 10- | -10 | -10 | -10 | -10
Negative Int | -200---20 | -201 | -201 | -200 | -200 | -200
  |   | -19 | -19 | -20 | -20 | -20
  |   | -50 | -50 | -50 | -50 | -50
  |   | - | - | -200 | -200 | -200
  |   | . |   | -200 | -200 | -200
  |   | -. | - | -200 | -200 | -200
  |   | -20- | 20 | -20 | -20 | -20
  |   | 20- | -20 | -20 | -20 | -20



</body>

</html>

## Why It's Good For The Game
Even though not used often on TG code, the modal inputs are used on
other code bases and inputting numbers has been a hell if it was
anything other than a positive integer. This PR aims to make the
handling as user friendly as possible, even if it's a lot more edge case
checking than the very simple code beforehand.
## Changelog
fixes #81457
🆑
qol: makes the tgui_input_number user friendly for negative and decimal
inputs
code: the onBlur={(_, value) => onBlur(value)} event should now be used
on all uses of RestrictedInput to ensure that the number is fully
sanitized whenever the user leaves the field or submits it through a
button
/🆑

* Makes the restricted number input user friendlier

---------

Co-authored-by: NovaBot <[email protected]>
Co-authored-by: Kashargul <[email protected]>
  • Loading branch information
3 people authored Feb 19, 2024
1 parent 3a2bdfe commit 840d5ed
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 5 deletions.
129 changes: 126 additions & 3 deletions tgui/packages/tgui/components/RestrictedInput.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,119 @@ import { Box } from './Box';
const DEFAULT_MIN = 0;
const DEFAULT_MAX = 10000;

/**
* Sanitize a number without interfering with writing negative or floating point numbers.
* Handling dots and minuses in a user friendly way
* @param value {String}
* @param minValue {Number}
* @param maxValue {Number}
* @param allowFloats {Boolean}
* @returns {String}
*/
const softSanitizeNumber = (value, minValue, maxValue, allowFloats) => {
const minimum = minValue || DEFAULT_MIN;
const maximum = maxValue || maxValue === 0 ? maxValue : DEFAULT_MAX;

let sanitizedString = allowFloats
? value.replace(/[^\-\d.]/g, '')
: value.replace(/[^\-\d]/g, '');

if (allowFloats) {
sanitizedString = maybeLeadWithMin(sanitizedString, minimum);
sanitizedString = keepOnlyFirstOccurrence('.', sanitizedString);
}
if (minValue < 0) {
sanitizedString = maybeMoveMinusSign(sanitizedString);
sanitizedString = keepOnlyFirstOccurrence('-', sanitizedString);
} else {
sanitizedString = sanitizedString.replaceAll('-', '');
}
if (minimum <= 1 && maximum >= 0) {
return clampGuessedNumber(sanitizedString, minimum, maximum, allowFloats);
}
return sanitizedString;
};

/**
* Clamping the input to the restricted range, making the Input smart for min <= 1 and max >= 0
* @param softSanitizedNumber {String}
* @param allowFloats {Boolean}
* @returns {string}
*/
const clampGuessedNumber = (
softSanitizedNumber,
minValue,
maxValue,
allowFloats,
) => {
let parsed = allowFloats
? parseFloat(softSanitizedNumber)
: parseInt(softSanitizedNumber, 10);
if (
!isNaN(parsed) &&
(softSanitizedNumber.slice(-1) !== '.' || parsed < Math.floor(minValue))
) {
let clamped = clamp(parsed, minValue, maxValue);
if (parsed !== clamped) {
return String(clamped);
}
}
return softSanitizedNumber;
};

/**
* Translate x- to -x and -x- to x
* @param string {String}
* @returns {string}
*/
const maybeMoveMinusSign = (string) => {
let retString = string;
// if minus sign is present but not first
let minusIdx = string.indexOf('-');
if (minusIdx > 0) {
string = string.replace('-', '');
retString = '-'.concat(string);
} else if (minusIdx === 0) {
if (string.indexOf('-', minusIdx + 1) > 0) {
retString = string.replaceAll('-', '');
}
}
return retString;
};

/**
* Translate . to min. or .x to mim.x or -. to -min.
* @param string {String}
*/
const maybeLeadWithMin = (string, min) => {
let retString = string;
let cuttedVal = Math.sign(min) * Math.floor(Math.abs(min));
if (string.indexOf('.') === 0) {
retString = String(cuttedVal).concat(string);
} else if (string.indexOf('-') === 0 && string.indexOf('.') === 1) {
retString = cuttedVal + '.'.concat(string.slice(2));
}
return retString;
};

/**
* Keep only the first occurrence of a string in another string.
* @param needle {String}
* @param haystack {String}
* @returns {string}
*/
const keepOnlyFirstOccurrence = (needle, haystack) => {
const idx = haystack.indexOf(needle);
const len = haystack.length;
let newHaystack = haystack;
if (idx !== -1 && idx < len - 1) {
let trailingString = haystack.slice(idx + 1, len);
trailingString = trailingString.replaceAll(needle, '');
newHaystack = haystack.slice(0, idx + 1).concat(trailingString);
}
return newHaystack;
};

/**
* Takes a string input and parses integers or floats from it.
* If none: Minimum is set.
Expand Down Expand Up @@ -37,14 +150,24 @@ export class RestrictedInput extends Component {
editing: false,
};
this.handleBlur = (e) => {
const { maxValue, minValue, onBlur, allowFloats } = this.props;
const { editing } = this.state;
if (editing) {
this.setEditing(false);
}
const safeNum = getClampedNumber(
e.target.value,
minValue,
maxValue,
allowFloats,
);
if (onBlur) {
onBlur(e, +safeNum);
}
};
this.handleChange = (e) => {
const { maxValue, minValue, onChange, allowFloats } = this.props;
e.target.value = getClampedNumber(
e.target.value = softSanitizeNumber(
e.target.value,
minValue,
maxValue,
Expand Down Expand Up @@ -149,7 +272,7 @@ export class RestrictedInput extends Component {

render() {
const { props } = this;
const { onChange, onEnter, onInput, value, ...boxProps } = props;
const { onChange, onEnter, onInput, onBlur, value, ...boxProps } = props;
const { className, fluid, monospace, ...rest } = boxProps;
return (
<Box
Expand All @@ -170,7 +293,7 @@ export class RestrictedInput extends Component {
onBlur={this.handleBlur}
onKeyDown={this.handleKeyDown}
ref={this.inputRef}
type="number"
type="number | string"
/>
</Box>
);
Expand Down
10 changes: 8 additions & 2 deletions tgui/packages/tgui/interfaces/NumberInputModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,12 @@ export const NumberInputModal = (props) => {
<Box color="label">{message}</Box>
</Stack.Item>
<Stack.Item>
<InputArea input={input} onClick={setValue} onChange={setValue} />
<InputArea
input={input}
onClick={setValue}
onChange={setValue}
onBlur={setValue}
/>
</Stack.Item>
<Stack.Item>
<InputButtons input={input} />
Expand All @@ -71,7 +76,7 @@ export const NumberInputModal = (props) => {
const InputArea = (props) => {
const { act, data } = useBackend<NumberInputData>();
const { min_value, max_value, init_value, round_value } = data;
const { input, onClick, onChange } = props;
const { input, onClick, onChange, onBlur } = props;

return (
<Stack fill>
Expand All @@ -92,6 +97,7 @@ const InputArea = (props) => {
minValue={min_value}
maxValue={max_value}
onChange={(_, value) => onChange(value)}
onBlur={(_, value) => onBlur(value)}
onEnter={(_, value) => act('submit', { entry: value })}
value={input}
/>
Expand Down

0 comments on commit 840d5ed

Please sign in to comment.