[EuiNumberField] Invalid step tooltip does not always show up for invalid values

[dej611]

Describe the bug

When using a <EuiNumberField step={1} .../> I would expect the native helping tooltip to show up together with the warning sign (on hover), but it doesn't work all the times.

In Safari it does not work at all. In Chrome I've found that it works intermittently depending on the amount of props passed to the component. In Firefox it works ok in all cases I've tested in Lens.

[JasonStoltz]

@dej611 What version of EUI is this?

[dej611]

I've tested on latest deployed on the EUI website and on Kibana here: https://github.com/elastic/kibana/pull/155208

[breehall]

It looks like the browser behavior is a bit inconsistent here between Safari, Chrome, and Firefox. I created a simple CodeSandbox demo.

In Safari, when the step is set to 2, and an invalid value is entered (like 3), I the tooltip when hovering over the invalid symbol doesn't show the warning message.

In Chrome, with the same invalid input, I can only see the warning message on the tooltip sometimes. It does not reappear when removing focus from the input and selecting it again.

In Firefox, I can see the warning message once. Just like Chrome, it does not reappear if I lost focus on the input and focus again.

[cee-chen]

In general, EUI should not be in the business of fixing browsers and browser-level APIs.

It's incredibly unlikely that we'll bake anything by default into EuiFieldNumber to try and polyfill this behavior, but if absolutely needed by consumers, you could do so on your own with an onKeyUp handler to obtain .validationMessage state and passing that to EuiFormRow's errors display.

Example CodeSandbox: https://codesandbox.io/s/hungry-cerf-m14fpn?file=/demo.js

In general, the absolute best validation is to perform your own and pass your own errors in based on your desired input.

[dej611]

From a quick check another alternative I found was to explicitly call the reportValidity method on the onChange callback.

const onChange = (e) => {
    setValue(e.target.value);
    if(!e.target.checkValidity()){
      e.target.reportValidity();
    }
  };

the result is a bit invasive but at least is cross-browser consistent.

Safari:

Chrome:

Firefox:

Edge (Chromium):

[cee-chen]

Super interesting and awesome use of the validity API!

On a separate note, that's interesting that onChange worked for you - in https://github.com/facebook/react/issues/16554#issuecomment-524590902, many people people reported that onChange fails to fire (as opposed to onKeyUp) on invalid values, e.g. when entering letters into an input type="number" in Safari/FF.

@dej611, are you thinking we could/should bake reportValidity into EuiFieldNumber by default? I'm not entirely opposed to it as it's certainly more helpful than just an icon with no message for Safari, but like you said, the UI/UX is a bit jarring - if we did implement it, I'm wondering if we should allow it to be turned off with a flag.

[dej611]

I'm wondering if we should allow it to be turned off with a flag

As a component consumer I would like this as well.

[cee-chen]

@dej611 I spiked this out a bit, but incredibly unfortunately, all webkit browsers (Safari/Chrome/FF) have a side effect where reportValidity causes autofocus on the invalid input. This is fine for keyboard/change events where the input is already focused, but incredibly problematic for onBlur events. I wanted reportValidity to fire on blur because browsers also handle undefined step validity on blur, and also because it helps make the messages slightly less ephemeral. However, what happens when calling reportValidity on blur is that the user becomes fully focus trapped in the input and can't escape 😱

To be perfectly honest with you, I'm incredibly leery of continuing further down this road (trying to polyfill/fix/smooth over default browser behavior). I think the biggest problem that started this whole mess is that we conflated consumer isInvalid state and styling with browser/native invalid state via the :invalid selector:

https://github.com/elastic/eui/blob/0d31625d8bc461f961293202d975e1f1e32b15a1/src/global_styling/mixins/_form.scss#L227-L229

TBH, my 2c is that we either should have baked in showing default browser error messages from the get-go (which would require extra context and wiring between EuiFormRow and the child input), or we should have kept invalid styling as coming purely from a custom --invalid class and not from browser :invalid. Instead we're now stuck in this awful in-between state where the browser sometimes reports invalid but the consumer doesn't, and the browser's error reporting doesn't match consumer error reporting. We should either leave invalid/error states up to the consumer, or fully support browser form validation with EUI styling (as opposed to browser styling), instead of half-heartedly trying to do both.

BTW, I really appreciate this report still as it helped me debug/drill down into why step behaved so dang weirdly/unexpectedly. I'm going to go back to the team to discuss this which approach we want to take (removing showing browser validation completely or leaning further into it).

[cee-chen]

Closing this issue in favor of https://github.com/elastic/eui/issues/6802. If we lean further into surfacing browser validity messages, we'll want to do so holistically (across all validatable form components, not just EuiFieldNumber), and we'll also likely want to integrate those messages into EuiFormRow as a more consistent UX across EUI.