From 2b64d703523a2856bd41b894dff9fcd5dc4bdc75 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Fri, 26 May 2023 19:24:10 +0200 Subject: [PATCH] generalize implementation to skip input value update when contents are "equivalent" in a given context, e.g. for numeric values with potentially different formatting in number fields --- modules/ui/fields/input.js | 46 +++++++++++++++++------------------ modules/util/get_set_value.js | 14 +++++++---- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/modules/ui/fields/input.js b/modules/ui/fields/input.js index b5f48049a..427265461 100644 --- a/modules/ui/fields/input.js +++ b/modules/ui/fields/input.js @@ -11,6 +11,7 @@ import { svgIcon } from '../../svg/icon'; import { cardinal } from '../../osm/node'; import { uiLengthIndicator } from '..'; import { uiTooltip } from '../tooltip'; +import { isEqual } from 'lodash-es'; export { uiFieldText as uiFieldColour, @@ -461,9 +462,20 @@ export function uiFieldText(field, context) { const vals = getVals(tags); const isMixed = vals.size > 1; var val = vals.size === 1 ? [...vals][0] : ''; - var shouldUpdate = true; + var shouldUpdate; if (field.type === 'number' && val) { + var numbers = val.split(';'); + var oriNumbers = utilGetSetValue(input).split(';'); + if (numbers.length !== oriNumbers.length) shouldUpdate = true; + numbers = numbers.map(function(v) { + v = v.trim(); + var num = Number(v); + if (!isFinite(num) || v === '') return v; + const fractionDigits = v.includes('.') ? v.split('.')[1].length : 0; + return formatFloat(num, fractionDigits); + }); + val = numbers.join(';'); // for number fields, we don't want to override the content of the // input element with the same number using a different formatting // (e.g. when entering "1234.5", this should not be reformatted to @@ -472,30 +484,18 @@ export function uiFieldText(field, context) { // but if the actual numeric value of the field has changed (e.g. // by pressing the +/- buttons or using the raw tag editor), we // can and should update the content of the input element. - shouldUpdate = false; - var numbers = val.split(';'); - var oriNumbers = utilGetSetValue(input).split(';'); - if (numbers.length !== oriNumbers.length) shouldUpdate = true; - numbers = numbers.map(function(v, idx) { - v = v.trim(); - var num = Number(v); - var oriNumber = oriNumbers[idx] || ''; - if (!isFinite(num) || v === '') { - if (v !== oriNumber) shouldUpdate = true; - return v; - } - oriNumber = likelyRawNumberFormat.test(oriNumber) ? parseFloat(oriNumber) : parseLocaleFloat(oriNumber); - if (num !== oriNumber) shouldUpdate = true; - const fractionDigits = v.includes('.') ? v.split('.')[1].length : 0; - return formatFloat(num, fractionDigits); - }); - val = numbers.join(';'); + shouldUpdate = (inputValue, setValue) => { + const inputNums = inputValue.split(';').map(setVal => + likelyRawNumberFormat.test(setVal) + ? parseFloat(setVal) + : parseLocaleFloat(setVal) + ); + const setNums = setValue.split(';').map(parseLocaleFloat); + return !isEqual(inputNums, setNums); + }; } - if (shouldUpdate) { - utilGetSetValue(input, val); - } - input + utilGetSetValue(input, val, shouldUpdate) .attr('title', isMixed ? [...vals].join('\n') : undefined) .attr('placeholder', isMixed ? t('inspector.multiple_values') : (field.placeholder() || t('inspector.unknown'))) .classed('mixed', isMixed); diff --git a/modules/util/get_set_value.js b/modules/util/get_set_value.js index ee9d5e0c5..597fa9b7c 100644 --- a/modules/util/get_set_value.js +++ b/modules/util/get_set_value.js @@ -1,14 +1,14 @@ // Like selection.property('value', ...), but avoids no-op value sets, // which can result in layout/repaint thrashing in some situations. /** @returns {string} */ -export function utilGetSetValue(selection, value) { - function setValue(value) { +export function utilGetSetValue(selection, value, shouldUpdate) { + function setValue(value, shouldUpdate) { function valueNull() { delete this.value; } function valueConstant() { - if (this.value !== value) { + if (shouldUpdate(this.value, value)) { this.value = value; } } @@ -17,7 +17,7 @@ export function utilGetSetValue(selection, value) { var x = value.apply(this, arguments); if (x === null || x === undefined) { delete this.value; - } else if (this.value !== x) { + } else if (shouldUpdate(this.value, x)) { this.value = x; } } @@ -39,5 +39,9 @@ export function utilGetSetValue(selection, value) { return selection.property('value'); } - return selection.each(stickyCursor(setValue(value))); + if (shouldUpdate === undefined) { + shouldUpdate = (a, b) => a !== b; + } + + return selection.each(stickyCursor(setValue(value, shouldUpdate))); }