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

[field controls] Respect validationMode #1053

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

atomiks
Copy link
Contributor

@atomiks atomiks commented Dec 12, 2024

Closes #1050

@mui-bot
Copy link

mui-bot commented Dec 12, 2024

Netlify deploy preview

https://deploy-preview-1053--base-ui.netlify.app/

Generated by 🚫 dangerJS against e101083

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 15, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 21, 2025
Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit 9681061
🔍 Latest deploy log https://app.netlify.com/sites/base-ui/deploys/67a1bb115e1f830008febc6a
😎 Deploy Preview https://deploy-preview-1053--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 23, 2025
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 23, 2025
@atomiks atomiks marked this pull request as ready for review January 24, 2025 03:59
@@ -7,6 +7,7 @@ import { CheckboxGroupContext } from './CheckboxGroupContext';
import type { FieldRoot } from '../field/root/FieldRoot';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think it's directly related to this PR, but I couldn't make a required checkbox group because required is a prop on individual checkboxes but not on CheckboxGroup:

// a combination of the CheckboxGroup and Field demos
export default function FieldCheckbox() {
  return (
    <Field.Root>
      <Field.Label>Pick one or more</Field.Label>

      <CheckboxGroup defaultValue={[]}>
        <label>
          <Checkbox.Root name="fuji-apple">
            <Checkbox.Indicator>
              <CheckIcon />
            </Checkbox.Indicator>
          </Checkbox.Root>
          Fuji
        </label>

        <label>
          <Checkbox.Root name="gala-apple">
            <Checkbox.Indicator>
              <CheckIcon />
            </Checkbox.Indicator>
          </Checkbox.Root>
          Gala
        </label>

        <label>
          <Checkbox.Root
            name="granny-smith-apple"
          >
            <Checkbox.Indicator>
              <CheckIcon />
            </Checkbox.Indicator>
          </Checkbox.Root>
          Granny Smith
        </label>
      </CheckboxGroup>

      <Field.Error match="valueMissing">
        You must pick at least one
      </Field.Error>
    </Field.Root>
  );
}

@@ -9,6 +9,8 @@ import { useDirection } from '../direction-provider/DirectionContext';
import { useRadioGroup } from './useRadioGroup';
Copy link
Member

@mj12albert mj12albert Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://codesandbox.io/p/sandbox/sweet-brattain-q9ptqr

When I make a required RadioGroup that has no initially checked radio and tab through it without checking any of the radios, it doesn't trigger an error state (I logged some stuff and it seems the validation functions are being called)

Also when the required RadioGroup is wrapped in a Form (with a submit button), if I click the submit button without touching the RadioGroup the error doesn't appear which could be the same issue?

However when a radio is checked, the console.log in the Form's submit handler doesn't log the value 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these bugs (with CheckboxGroup as well) related to this PR or are they on master as well? If on master, we can create a new issue for them to separate the work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Field] Base UI components don't respect validationMode consistently
3 participants