Explain Firefox how numeric input should work #1600

Karakatiza666 · 2024-04-01T23:08:07Z

Is this a user-visible change (yes/no): yes

Fix #1522 : WebConsole: numeric inputs allow arbitrary text input in Firefox
Fix #1748 : WebConsole: Invariant failed setting non nullable value to null

gz · 2024-04-03T17:06:11Z

In this video I'm trying to clear the field by hitting backspace but it doesn't clear the last 4 digits in the field

Screencast.from.04-03-2024.10.04.52.AM.webm

gz · 2024-04-03T17:07:04Z

I'm trying to select the field and overwrite it with a single number but it doesn't let me

Screencast.from.04-03-2024.10.06.28.AM.webm

Karakatiza666 · 2024-04-03T17:07:55Z

The case for the first behavior is the minimum allowed value for that librdkafka option

Karakatiza666 · 2024-04-03T17:09:24Z

I need to update this behavior to enable starting to write a new number even though it doesn't fit in the allowed range in the beginning

mihaibudiu · 2024-04-03T17:09:25Z

The case for the first behavior is the minimum allowed value for that librdkafka option

Validation should be done at the end, not on every keystroke.
How can you type the number 2000?

gz · 2024-04-03T17:09:40Z

The case for the first behavior is the minimum allowed value for that librdkafka option

I can't set it e.g., 1001 since it doesn't allow me to change the first digits inthe sequence

gz · 2024-04-03T17:11:21Z

It also feels more sluggish to me when I try this in the browser.

How about we do the following: let users enter any values, including non-digit characters but highlight the form red when it's invalid and display what the error is similar to how the other form elements do it (e.g., table input or forms for connectors)?

gz · 2024-04-03T16:53:34Z

CHANGELOG.md

@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed

 - WebConsole: State shows InQueue when program is empty (#1443)
+- WebConsole: Fix behavior of numeric inputs in some browsers, including Firefox


Suggested change

- WebConsole: Fix behavior of numeric inputs in some browsers, including Firefox

- WebConsole: Fix behavior of numeric inputs in Firefox

gz · 2024-04-03T16:55:57Z

web-console/src/lib/components/input/NumberInput.tsx

+import { TextField, TextFieldProps } from '@mui/material'
+
+export const handleNumericKeyDown: KeyboardEventHandler<Element> = event => {
+  if (/Key[ACVXYZ]/.test(event.code) && event.ctrlKey) {


what does this do why ACVXYZ

maybe add a comment

Will add a comment. Shortcuts like Ctrl + C, Ctrl + V etc.

gz · 2024-04-03T16:57:11Z

web-console/src/lib/components/input/NumberInput.tsx

+  return number
+}
+
+export const numberInputProps = (


what does this function do? comment?

gz · 2024-04-03T16:57:57Z

web-console/src/lib/components/services/dialogs/KafkaService.tsx

+                        component={NumberInput as any}
+                        {...(([, min, max]) => {
+                          return { min: parseInt(min), max: parseInt(max) }
+                        })(fieldOptions[field].range.match(/(\d+) .. (\d+)/) ?? [])}


.. why are there two spaces?

This is an implementation detail related to file librdkafkaOptions.ts, I will move it to encapsulate it there, rather than expose at usage-time

gz · 2024-04-03T16:59:12Z

web-console/src/lib/components/streaming/import/SQLValueInput.tsx

            ...(({ min, max }) => ({ min: min.toNumber(), max: max.toNumber() }))(numericRange(columnType)),
-            value: props.value === null ? '' : props.value, // Enable clearing of the input when setting value to null
+            value: props.value as null | number,


I don't understand what this expression does? A cast and then bitwise or?

A type cast (doesn't affect the runtime value) to a type "number or null"

then you should write (null | number)

Is there an Option type? That would be much better.

Not a native one. A user-space wrapper class could be used, but the native null works just as well - the typing is strong everywhere, so null-ability is explicit and can not be ignored

The check for unguarded access to values that can be null and undefined is also enforced by typescript compiler, not just editor type highlight

as (null | number) gets reduced by the formatter to as null | number

gz · 2024-04-03T23:50:21Z

Thanks, looks like behavior works as intended now. I fear there is still some confusing behavior for a user:

it seems to reset the back to a default value when switching views if the text field is currently red, this is surprising for a user if input just "corrects" itself

Screencast.from.04-03-2024.04.39.47.PM.webm

It looks like the UI knows about the min or max value of each field but it doesn't do a good job of communicating it back to the user (aside from making the text field red) e.g., how about we display what range is accepted as a small error text below the form field (like we do in other cases like in the screenshot)
Screencast from 04-03-2024 04:45:47 PM.webm
Then even though an integer form field has errors, I am able to submit it (and it reverts it just deletes the option), I guess this is expected, but a little surprising that it lets me submit a form with errors (I'd have expected it to bring me back to that dialog as it does when for example you leave the bootstrap server field blank)
Screencast from 04-03-2024 04:43:09 PM.webm

gz · 2024-04-05T16:41:37Z

There is some odd behavior, the first time a form is filled out the min/max range is not displayed but the text cell becomes red, after I submit the form the ranges are displayed

Screencast.from.04-05-2024.09.40.08.AM.webm

gz · 2024-04-05T16:44:52Z

Given how many pitfalls there were with this PR: lets add test-cases for these cases before we merge (I don't see any currently). We should get into the habit to have tests for it otherwise long term stability and evolution of it will suffer.

Karakatiza666 · 2024-05-14T02:01:19Z

Updated the PR. In some cases quickly typing up numbers gets laggy - that is due to implementation that depends on attempting to parse the input as a number on each keystroke. This is a tradeoff for a simpler and more robust implementation and can be optimized later if needed.

Signed-off-by: George <bulakh.96@gmail.com>

Add comments to implementation of NumberInput Signed-off-by: George <bulakh.96@gmail.com>

Add invalid range as a condition for a form failure Add error message when a number in a form is out of range Signed-off-by: George <bulakh.96@gmail.com>

Signed-off-by: George <bulakh.96@gmail.com>

Karakatiza666 added bug Something isn't working Web Console Related to the browser based UI javascript Pull requests that update Javascript code User-facing For PRs that lead to Feldera-user visible changes labels Apr 1, 2024

Karakatiza666 requested a review from gz April 1, 2024 23:08

Karakatiza666 force-pushed the number-input branch 3 times, most recently from e4b734c to 9e49bd3 Compare April 3, 2024 16:27

gz reviewed Apr 3, 2024

View reviewed changes

Karakatiza666 force-pushed the number-input branch 3 times, most recently from b967b71 to cb2c3ef Compare April 3, 2024 23:40

Karakatiza666 force-pushed the number-input branch 2 times, most recently from 01ed2f8 to 233003c Compare April 4, 2024 20:51

Karakatiza666 force-pushed the number-input branch 6 times, most recently from b2b414e to ff2c691 Compare May 14, 2024 01:56

Karakatiza666 force-pushed the number-input branch from ff2c691 to c620e98 Compare May 14, 2024 02:04

gz self-requested a review May 22, 2024 16:53

gz approved these changes May 22, 2024

View reviewed changes

Karakatiza666 added 7 commits May 23, 2024 12:20

Explain Firefox how numeric input should work

489d7a2

Signed-off-by: George <bulakh.96@gmail.com>

Enable typing up numbers from the start when value range is limited

ebb3c61

Add comments to implementation of NumberInput Signed-off-by: George <bulakh.96@gmail.com>

Fix invalid NumberInput value resetting by itself

58a37f3

Add invalid range as a condition for a form failure Add error message when a number in a form is out of range Signed-off-by: George <bulakh.96@gmail.com>

Enable form validation on input change, not after submit

6e6fe67

Signed-off-by: George <bulakh.96@gmail.com>

Merge fixes

7775665

Signed-off-by: George <bulakh.96@gmail.com>

Refactor number input again, add Unit test

42c0ccb

Signed-off-by: George <bulakh.96@gmail.com>

Use the same test sequence for NumberInput and BigNumberInput

228eff0

Signed-off-by: George <bulakh.96@gmail.com>

Karakatiza666 force-pushed the number-input branch from 55024c5 to 228eff0 Compare May 23, 2024 12:25

Karakatiza666 merged commit 2029590 into main May 23, 2024
5 checks passed

Karakatiza666 deleted the number-input branch May 23, 2024 12:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explain Firefox how numeric input should work #1600

Explain Firefox how numeric input should work #1600

Karakatiza666 commented Apr 1, 2024 •

edited

gz commented Apr 3, 2024

gz commented Apr 3, 2024 •

edited

Karakatiza666 commented Apr 3, 2024 •

edited

Karakatiza666 commented Apr 3, 2024 •

edited

mihaibudiu commented Apr 3, 2024

gz commented Apr 3, 2024

gz commented Apr 3, 2024 •

edited

gz Apr 3, 2024

gz Apr 3, 2024

gz Apr 3, 2024

Karakatiza666 Apr 3, 2024

gz Apr 3, 2024

gz Apr 3, 2024

Karakatiza666 Apr 3, 2024 •

edited

gz Apr 3, 2024

Karakatiza666 Apr 3, 2024 •

edited

mihaibudiu Apr 3, 2024

mihaibudiu Apr 3, 2024

Karakatiza666 Apr 3, 2024

Karakatiza666 Apr 3, 2024 •

edited

Karakatiza666 Apr 3, 2024

gz commented Apr 3, 2024

gz commented Apr 5, 2024

gz commented Apr 5, 2024

Karakatiza666 commented May 14, 2024

	- WebConsole: Fix behavior of numeric inputs in some browsers, including Firefox
	- WebConsole: Fix behavior of numeric inputs in Firefox

Explain Firefox how numeric input should work #1600

Explain Firefox how numeric input should work #1600

Conversation

Karakatiza666 commented Apr 1, 2024 • edited

gz commented Apr 3, 2024

gz commented Apr 3, 2024 • edited

Karakatiza666 commented Apr 3, 2024 • edited

Karakatiza666 commented Apr 3, 2024 • edited

mihaibudiu commented Apr 3, 2024

gz commented Apr 3, 2024

gz commented Apr 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Karakatiza666 Apr 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Karakatiza666 Apr 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Karakatiza666 Apr 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gz commented Apr 3, 2024

gz commented Apr 5, 2024

gz commented Apr 5, 2024

Karakatiza666 commented May 14, 2024

Karakatiza666 commented Apr 1, 2024 •

edited

gz commented Apr 3, 2024 •

edited

Karakatiza666 commented Apr 3, 2024 •

edited

Karakatiza666 commented Apr 3, 2024 •

edited

gz commented Apr 3, 2024 •

edited

Karakatiza666 Apr 3, 2024 •

edited

Karakatiza666 Apr 3, 2024 •

edited

Karakatiza666 Apr 3, 2024 •

edited