Let the user edit the font size settings with the keyboard

[AlbyIanna]

Motivation

It's difficult for the user to edit the font size in the preferences using the keyboard.

On the current state, when a user tries to edit the value of the font size with the keyboard, the value would update unexpectedly.

For example:

• the font size is 72
• focus the input field
• press BACKSPACE
• expected: the font size is 7; result: the font size is 8 😢
• now press 1
• expected: the font size is 81; result: the font size is 72 😢

The reason is we validate the user input as they type, and if the input is invalid, we change the value to a valid one.

Change description

With this change, I want to validate the user input as they finished typing instead of as they type. In order to achieve that, I used the onBlur callback of the input field, so that the user can type whatever they want in the input field (only numbers, to be precise), and as soon as their focus change from the input field to another element (e.g.: they click on any other piece of UI or press ESCAPE) we validate the input they just typed in and, if necessary, change it to a valid value.

Before the change (video):

Screen.Recording.2022-10-07.at.18.26.17.mov

After the change (video):

Screen.Recording.2022-10-07.at.18.26.43.mov

Other information

Fixes #1542

Reviewer checklist

• PR addresses a single concern.
• The PR has no duplicates (please search among the Pull Requests before creating one)
• PR title and description are properly filled.
• Docs have been added / updated (for bug fixes / features)

[davegarthsimpson]

unfortunately there are a few issues here:

1. This breaks the arrow buttons functionality in general due to the wrong value being referenced in the onStep function
2. I believe it's makes things more predictable / cleaner to cast the value to string only in the JSX and to number only in the onChange handler (ultimately, only where really necessary), then record "empty string" as an exception
3. It results in a buggy feeling UX if the user clicks an arrow when the input value is '0' or '-' (the blur restores the initial value and then "steps it"), in the below changes I've made sure to disable the steppers when an invalid value is in the input
4. the value is passed as prop from the parent, when it changes it should reset the internal state, we should use the concept of an initialValue together with a key attribute in the stepper JSX to achieve this, the internal state does not need to be updated when setSettingsStateValue is invoked as doing should trigger a reset anyway

I pushed some changes to a different branch here which resolve the above:
ce3b47b
There is also a small styling change as I noticed the position of the arrow buttons container was wrong in the font size stepper.

[francescospissu]

It works well for me. The only small problem I encountered occurs when an invalid value is entered in the input field when the current value is the minimum (font size 8), in this case the input value is not automatically updated as entering an invalid value while the current value is greater than the minimum.

See the video:

sketch_oct19a._.Arduino.IDE.2.0.1.2022-10-19.15-22-16.mp4

[AlbyIanna]

It works well for me. The only small problem I encountered occurs when an invalid value is entered in the input field when the current value is the minimum (font size 8), in this case the input value is not automatically updated as entering an invalid value while the current value is greater than the minimum.

Nice catch! I just pushed a fix 👍

[kittaakos]

I did not check the code changes thoroughly, but the behavior only. If I set the font size to 73 which exceeds 72 and hit enter, the new value will be 72. If I set the value to 0, nothing changes. I expected symmetrical behavior; setting the value to 0 will set the font size to min; 8.

[davegarthsimpson]

 const setValidValue = (value: number): void => {
    const clampedValue = clamp(value, minValue, maxValue);
    setValueState({ currentValue: clampedValue, isEmptyString: false });
    setSettingsStateValue(clampedValue);
  };

this doesn't feel right, if the value has changed setSettingsStateValue resets the whole component as well as the internal state (because it changes the jsx key), thus setting the internal state shouldn't be necessary, if we're having to do this things are likely muddled and we probably need to rethink the lifecycle

[kittaakos]

It works for me. Thank you!

[kittaakos]

No newline at the end of file

[AlbyIanna]

this doesn't feel right, if the value has changed setSettingsStateValue resets the whole component as well as the internal state (because it changes the jsx key), thus setting the internal state shouldn't be necessary, if we're having to do this things are likely muddled and we probably need to rethink the lifecycle

Ok, I'll look into it better later 👍

[AlbyIanna]

@francescospissu would you please give it another try? @davegarthsimpson just pushed a fix.

[francescospissu]

Now it works perfectly for me. Thanks @AlbyIanna 🙂