Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] - Add validationBehavior support for Select #3913

Open
Rain-YuXia opened this issue Oct 18, 2024 · 13 comments · May be fixed by #4281
Open

[BUG] - Add validationBehavior support for Select #3913

Rain-YuXia opened this issue Oct 18, 2024 · 13 comments · May be fixed by #4281
Labels
📦 Scope : Components Related to the components 🐛 Type: Bug Something isn't working ✨ Type: Enhancement New enhancement on existing codebase

Comments

@Rain-YuXia
Copy link

NextUI Version

2.4.8

Describe the bug

Input and Select validation behaviour is different.

Your Example Website or App

No response

Steps to Reproduce the Bug or Issue

When I set isRequired on both Input & Select, the select dropdown uses HTML built-in validation. However, the input box doesn't.

From the screenshots below, you can see the validation error pops up for Select, but not for Input

Expected behavior

They should behave in the same way.

Screenshots or Videos

Screenshot 2024-10-18 at 2 43 12 PM Screenshot 2024-10-18 at 2 44 26 PM

Operating System Version

Mac

Browser

Chrome

Copy link

linear bot commented Oct 18, 2024

@wingkwong
Copy link
Member

Can you share the exact code of those components shown in the screenshot? A sandbox would be better.

@Rain-YuXia
Copy link
Author

Sorry I couldn't figure out how to work with a Sandbox lol... My code is more or less like below:

<form onSubmit={submit}>
    <Select
        label="Type of Property"
        labelPlacement="outside-left"
        isRequired
        aria-label="Type of Property"
        variant="bordered"
        placeholder="Select the Property Type"
        isInvalid={!!errors.property_type_id}
        errorMessage={errors.property_type_id}
        selectedKeys={[data.property_type_id]}
        onChange={(e) => {updateListingHandler('property_type_id', e.target.value)}}
    >
        {
            category.property_types.map((item) => (
                <SelectItem key={item.id}>
                    {item.name}
                </SelectItem>
            ))
        }
    </Select>

    <Input
          label="$ Per Annum"
          labelPlacement="outside-left"
          isRequired
          size="md"
          variant="bordered"
          startContent={
              <div className="pointer-events-none flex items-center">
                  <span className="text-default-400 text-small">$</span>
              </div>
          }
          value={formatNumber(data.value_per_annum)}
          onValueChange={(v) => updateListingHandler('value_per_annum', v)}
      />
</form>

Then when the form is submitting with empty data in Type of Property, it doesn't trigger onSubmit event and pop-up the built-in validation error.

However, if the $ per Annum is empty, no built-in validation triggered and the code can go to submit function.

Hope I made myself clear lol

@jijiseong
Copy link

jijiseong commented Oct 22, 2024

Try this.
The pop-up still doesn`t appear.
but focus and highlight work.

    <Input isRquired validationBehavior="native" />

@wingkwong
input uses domRef instead of the original ref.
So the pop-up doesn't appear.
Is this intentional?

// use-input.ts line 363

 const getInputProps: PropGetter = useCallback(
    (props = {}) => {
      return {
        ref: domRef,
        //...
      };
      //....
    },

    [
      //...
    ],
  );

@Rain-YuXia
Copy link
Author

Try this. The pop-up still doesn`t appear. but focus and highlight work.

    <Input isRquired validationBehavior="native" />

@wingkwong input uses domRef instead of the original ref. So the pop-up doesn't appear. Is this intentional?

// use-input.ts line 363

 const getInputProps: PropGetter = useCallback(
    (props = {}) => {
      return {
        ref: domRef,
        //...
      };
      //....
    },

    [
      //...
    ],
  );

Thanks for the reply.

But I prefer to disable the native validation behaviour for Select, since I have my own validation logics.

As I found out, Input and DatePicker both have disabled the native behaviour. However, Select hasn't. I think it's a inconsistency.

And setting validationBehavior="aria" on Select doesn't disable the built-in validation.

@wingkwong wingkwong added 🐛 Type: Bug Something isn't working 📦 Scope : Components Related to the components and removed 🔎 Status: More Info Needed labels Oct 23, 2024
@wingkwong
Copy link
Member

@jijiseong the original ref is passed to useDomRef. In select, there is a hidden native select / input where input component doesn't have it. Maybe the validation part is different here. Need to dig a bit to see if it is related.

@jijiseong
Copy link

jijiseong commented Oct 24, 2024

would you assign to me?
I'll try to fix it.

@wingkwong
Copy link
Member

@jijiseong assigned. thanks.

@DonBrowny
Copy link

@wingkwong What should i do to achieve the reverse of this issue, i.e disable the build in html validation from select.
For example:
image

For the above screenshot i used isRequired in Input but not on Select (but used isInvalid) on flipside of this appraoach is loose the asterisk in the select's label. Is there a better approach for this?

@jijiseong
Copy link

jijiseong commented Oct 28, 2024

@wingkwong
Please cancel the assignment. TT
I had tried to fix this issue, but I don't know what is the best way.
One thing that's certain is that it works properly when passing the original ref direlctly to the input prop.

 const getInputProps: PropGetter = useCallback(
    (props = {}) => {
      return {
        ref: originalProps.ref,
        //...
      };
      //....
    },

    [
      //...
    ],
  );

@chirokas
Copy link
Contributor

@hagai-slash-in
Copy link

I found that in order to enable the Input to have the default HTML required validation I just need to add required attribute to the Input (on top of the isRequired to get the visual indication).

When using select with isRequired, the generated HTML includes a hidden input with a required attribute, this is set to the value and submitted with the form:

<input tabindex="-1" required type="text" value="" style="font-size: 16px;">

I would expect the Input component to also add the required attribute when it is set with isRequired to enable native form support.

@ryo-manba
Copy link
Member

Select is implemented by customizing react-aria, so we haven't been able to incorporate validationBehavior yet.
Therefore, we need to enable validationBehavior support for Select.

@ryo-manba ryo-manba changed the title [BUG] - Validation Inconsistency [BUG] - Add validationBehavior support for Select Nov 6, 2024
@ryo-manba ryo-manba added the ✨ Type: Enhancement New enhancement on existing codebase label Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 Scope : Components Related to the components 🐛 Type: Bug Something isn't working ✨ Type: Enhancement New enhancement on existing codebase
Projects
None yet
7 participants