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

How to report Error with Checkboxes? #264

Open
awhitford opened this issue Nov 10, 2024 · 13 comments
Open

How to report Error with Checkboxes? #264

awhitford opened this issue Nov 10, 2024 · 13 comments
Assignees
Labels
question Further information is requested

Comments

@awhitford
Copy link

awhitford commented Nov 10, 2024

I am using Solid and I have a set of checkboxes:
image
There is a unique Valibot 🤖 rule that prevents certain combinations from being acceptable.
Specifically, you are unable to choose more than one of Preact, React, and Solid (because they have unique JSX configurations, so you may choose just one).
(These are checkboxes, and not radio buttons, because many combinations are acceptable.)

I have updated Valibot with a rule to prevent bad data. Rule looks something like:

export const uiFrameworks = ["preact", "react", "solid", "svelte", "vue"] as const
export const jsxFrameworks = uiFrameworks.filter((framework) => ["preact", "react", "solid"].includes(framework))

...
  uiFrameworks: v.pipe(
    v.array(v.picklist(uiFrameworks, "Invalid UI Framework.")),
    v.checkItems(
      (item, index, array) =>
        !jsxFrameworks.includes(item) || array.filter((value) => jsxFrameworks.includes(value)).length <= 1,
      "Multiple JSX based Frameworks are not allowed.",
    ),
  ),

I am pretty sure that Valibot is working because I tested the above in the playground, and I can prevent the form from submitting bad combinations.

However, I am struggling to provide error feedback back to the user.

Consider code like:

      <div class="mt-4">
        <div class="label">
          <span class="label-text font-bold md:text-lg">Which user interface frameworks would you like to use?</span>
        </div>
        <For each={uiFrameworks}>
          {(item) => (
            <Field name="uiFrameworks" type="string[]">
              {(field, props) => (
                <div class="form-control max-w-64">
                  <label class="label mx-10 cursor-pointer">
                    <span class="label-text">{capitalCase(item)}</span>
                    <input
                      {...props}
                      name="uiFrameworks"
                      type="checkbox"
                      value={item}
                      checked={field.value?.includes(item)}
                      class={`checkbox ${field.error ? "checkbox-error" : ""}`}
                    />
                  </label>
                  {field.error && (
                    <div class="label">
                      <span class="label-text-alt text-error">{field.error}</span>
                    </div>
                  )}
                </div>
              )}
            </Field>
          )}
        </For>

For broader context, this is what I am doing for the form:

export default function ConfigureForm() {
  const [configurationForm, { Form, Field }] = createForm<ConfigurationForm>({
    validate: valiForm(ConfigurationSchema),
  })

  const handleSubmit = async (values: ConfigurationForm) => {
    console.log("Form passed client validation with values:", values)
    const { data, error } = await actions.submitOrder(values)
    if (data?.success && data?.redirect) {
      navigate(data?.redirect)
    }
  }

It is weird in that the submit doesn't seem to trigger a re-render of the checkboxes.
And I've never seen field.error not be a blank string.

I double checked the docs and playground example, but I don't see a scenario talking about checkbox validation -- at least beyond a simple example like, "Click this to accept Terms and Conditions".

I appreciate any help. Thanks!

@fabian-hiller
Copy link
Owner

fabian-hiller commented Nov 11, 2024

Please have a look at this guide and let me know if it solves your problem: https://modularforms.dev/solid/guides/field-arrays

This code example might help you too: https://stackblitz.com/edit/modular-forms-solid?file=src/routes/todos.tsx

@fabian-hiller fabian-hiller self-assigned this Nov 11, 2024
@fabian-hiller fabian-hiller added the question Further information is requested label Nov 11, 2024
@awhitford
Copy link
Author

Using FieldArray does not seem appropriate because my checkboxes are a static list, not a dynamic list of todo's, for example. I'm just trying to give feedback for unacceptable combinations.

I was looking very closely at: https://stackblitz.com/edit/modular-forms-solid?file=src/routes/special.tsx
In fact, I restructured my code to mimic it. Alas, my problem requires Valibot to truly experience the issue.

As a result, I crafted a small project that demonstrates the problem. It uses pnpm and astro -- there is little styling, so it is ugly, but I think you can get the gist of what I'm trying to do. The project has a README that explains how to run it and reproduce the problem.

modular-forms-checkbox-bug.zip

@fabian-hiller
Copy link
Owner

Please provide a StackBlitz link with a minimal reproduction if you want me to take a look at it.

@awhitford
Copy link
Author

Please provide a StackBlitz link with a minimal reproduction if you want me to take a look at it.

Here you go: https://stackblitz.com/edit/github-wjzn2o?file=README.md
I appreciate your help. 🙏

@fabian-hiller
Copy link
Owner

fabian-hiller commented Nov 13, 2024

Here is a first fix: https://stackblitz.com/edit/github-wjzn2o-tgg3qn?file=src%2Fcomponents%2FConfigureForm.tsx%3AL109

Feel free to let me know your expected behaviour and I will try to teak it out for you.

@awhitford
Copy link
Author

That's interesting. If you consider that there are field errors and form errors, I guess I should have an overall form error displayed too. However, I wouldn't want to duplicate the field error like "missing project name" -- I see that it isn't, but I'm confused because it is outside the Field and For loop.

image

In other words, how is "Project Name is required." not part of configurationForm.response.message? 🤔

Having said that, I also expect the "bad checkbox" value would be marked as an error too. I made a tiny tweak to the code:
https://stackblitz.com/edit/github-wjzn2o-zd7uzd?file=src%2Fcomponents%2FConfigureForm.tsx

I simply added an explicit style to the label when there is a field error:

                <div style="padding-left: 1em">
                  <label style={field.error ? 'color: red' : ''}>
                    {item}
                    <input
                      {...props}
                      name="uiFrameworks"
                      type="checkbox"
                      value={item}
                      checked={field.value?.includes(item)}
                      class={`checkbox ${field.error ? 'checkbox-error' : ''}`}
                    />
                  </label>
                  {field.error && <div style="color: red">{field.error}</div>}
                </div>

My issue is that field.error is always an empty string, so this isn't highlighting the specific "bad" checkboxes.

image

You can see that the error messages are part of configurationForm.response.message, but the two conflicting checkbox values emit the same error message which doubles it up. I'm OK with emitting two error messages, as long as they bind to the bad checkbox values.

In other words, what I'm expecting to see is something like:

[x] Preact
    - Multiple JSX based Frameworks are not allowed.
[ ] React
[x] Solid
    - Multiple JSX based Frameworks are not allowed.
[ ] Svelte
[ ] Vue

@fabian-hiller
Copy link
Owner

In other words, how is "Project Name is required." not part of configurationForm.response.message? 🤔

Only error messages that are not related to a specific field are added to the form response.

The two conflicting checkbox values emit the same error message which doubles it up.

Here is a fix for only showing the error once: https://stackblitz.com/edit/github-wjzn2o-uqcajv?file=src%2Fcomponents%2FConfigureForm.tsx

I'm OK with emitting two error messages, as long as they bind to the bad checkbox values.

This only works if you use boolean instead of string[] or a field array, because then each field has its own state and therefore its own error message.

@awhitford
Copy link
Author

I don't understand how to use a Field Array for a checkbox. Perhaps you can share an example?

@fabian-hiller
Copy link
Owner

I think you're right, and my field array idea was a dump idea and can't be applied to this use case. Sorry about that. Do any of the above solutions work for you?

@awhitford
Copy link
Author

I'm going with your advice around configurationForm.response.message, but it is a little imperfect. It isn't a showstopper, but it feels like the solution could be better.

One thing I don't like is that my actual form is quite long (you need scroll vertically) -- the submit button is at the end. If I input invalid data into a text field, the form will scroll to the bad field. However, if I have a checkbox issue, the error will appear in red, but the user isn't scrolled to that area of the form. Again, I added a workaround, but ideally that would be better too.

Note that I am using @modular-forms/solid version 0.23.0 and valibot version 0.42.1. (Waiting for the Valibot 1.0 release before upgrading.)

@fabian-hiller
Copy link
Owner

Another workaround would be to add the checkboxes as booleans instead of an array of strings. That way each checkbox is its own field with its own error message.

So the auto-scrolling feature works for all fields except the checkboxes?

@awhitford
Copy link
Author

I'm unclear on how the auto-scroll is programmed. Is that a Modular Forms feature? But yeah, it isn't auto scrolling to the checkbox field with errors. I'm unclear why not.

@fabian-hiller
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants