Only set config value if it's different #363
Conversation
gsproston-scottlogic
requested review from
chriswilty,
pmarsh-scottlogic and
heatherlogan-scottlogic
| } | ||
| } | ||
|
|
||
| function focusLost(event: React.FocusEvent<HTMLDivElement>) { | ||
| const value = event.target.innerText.trim(); | ||
| // asynchronously set the configuration value | ||
| void setConfigurationValue(config.id, value); | ||
| setConfigurationValue(value); |
There was a problem hiding this comment.
This onBlur function would be completely unnecessary if we were using a native html input / textarea.
There was a problem hiding this comment.
Out of interest, would this not still be required with input and textareas? To set the value on focus loss?
Ordinarily I'd be happy to swap out the contenteditable div here, but seeing as I'm off next week and the actual original issue is solved by this PR, it makes more sense to me to just merge this in and create a new issue to use a textarea element. |
|
@gsproston-scottlogic I remain unconvinced... It's still a sticking plaster, which will be reverted in the subsequent issue, so we'd be "fixing" it twice. We shouldn't even be firing an event in the first place if the value hasn't changed. We could always prioritise the proper fix and tackle it early next week? |
Yeah I think that's fine. In that case would you mind updating the linked issue to include the contenteditable refactor? |
|
Created #371 for the more robust fix. |
gsproston-scottlogic
deleted the
354-defences-successfully-configured-message-every-time-an-input-loses-focus
branch
Added a check to see if the new config value is different before setting the new value in the backend.