colinaut / alpinejs-plugin-simple-validate

Simple Alpine form validation plugin
97 stars 4 forks source link

Validating stopped working #23

Closed kevinmu17 closed 6 months ago

kevinmu17 commented 6 months ago
    <form x-data x-validate @submit="$validate.submit">
        <div>
            <input type="email" name="message[testing]" required>
            <div class="error-msg">
                E-mailadres is required
            </div>
        </div>
        <button type="submit">send</button>
    </form>

When using name attributes like above i'm getting an error

Uncaught TypeError: Cannot read properties of null (reading 'setAttribute')

Line 26 const setAttr = (el, attr, value = "") => el.setAttribute(attr, value);

Also, when using 2 different forms, like in #9 the error messages get the same form ID. Wouldn't it be a better idea to add a class to the error-msg and query them from the form tag so you don't get ID conflicts?

  function toggleError(field, valid) {
    const parentNode = getData(field).parentNode;
    const errorMsgNode = getErrorMsgFromId(field);
    setAttr(field, "aria-invalid", !valid);
    if (valid) {
      setAttr(errorMsgNode, HIDDEN);
      parentNode.removeAttribute(DATA_ERROR);
    } else {
      errorMsgNode.removeAttribute(HIDDEN); // <-------- THIS GETS HIGHLIGHTED AS THE ERROR
      setAttr(parentNode, DATA_ERROR, errorMsgNode.textContent);
    }
  }
colinaut commented 6 months ago

Ok fixed in Release 1.7.25. I changed the code to make the error msg ids unique for each form so there shouldn't be an ID conflict now. Note, the error message needs a unique ID as it is used for the aria.

krismach commented 6 months ago

1.7.25 broke error messages using error-msg-id:

<div>
    <label for="firstName">First name</label>
    <div>
        <div>
            <input required data-error-msg="First name is required" id="firstName" name="firstName" type="text">
        </div>
        <div id="error-msg-firstName" class="error-msg">
            First name is required</div>
    </div>
</div>

1.7.24 and earlier, the above works as intended.

kevinmu17 commented 6 months ago

Thanks for diving in so quick! :) appreciate that!

I'm still getting the same errors after the update to v1.7.25:

Stacktrace;

Uncaught TypeError: Cannot read properties of null (reading 'setAttribute')
    at p (alpine.validate.esm.js:24:48)
    at j (alpine.validate.esm.js:305:7)
    at H (alpine.validate.esm.js:126:7)
    at HTMLSelectElement.q (alpine.validate.esm.js:291:27)
p @ alpine.validate.esm.js:24
j @ alpine.validate.esm.js:305
H @ alpine.validate.esm.js:126
q @ alpine.validate.esm.js:291

module.esm.js:551 Alpine Expression Error: Cannot read properties of null (reading 'removeAttribute')
Expression: "$validate.submit;"

alpine.validate.esm.js:308 Uncaught TypeError: Cannot read properties of null (reading 'removeAttribute')
    at j (alpine.validate.esm.js:308:20)
    at alpine.validate.esm.js:182:9
    at Array.forEach (<anonymous>)
    at z.submit (alpine.validate.esm.js:176:23)
    at lt (module.esm.js:643:24)
    at module.esm.js:631:9
    at Xr (module.esm.js:541:12)
    at module.esm.js:3304:5
    at r (module.esm.js:2604:25)
    at module.esm.js:2632:7
j @ alpine.validate.esm.js:308

alpine.validate.esm.js:308 Uncaught TypeError: Cannot read properties of null (reading 'removeAttribute')
    at j (alpine.validate.esm.js:308:20)
    at H (alpine.validate.esm.js:126:7)
    at HTMLInputElement.q (alpine.validate.esm.js:291:27)
colinaut commented 6 months ago

@krismach

Shoot you are right my last change breaks using the id. I have another way to fix the multiform issue which will also not break your issue

colinaut commented 6 months ago

@krismach The latest release 1.7.27 reverts so that custom error messages using ids should work again. Note there is a breaking change in that the error-msg id is based on the id for the field and not the name:error-msg-${id-of-matching-field}. You're example should still work fine since the id is the same. The reason for this change is that some people may have multiple forms with fields that have the same name, but ids are always unique.

@kevinmu17 I'm not sure if this new release fixes your issue or not. It should but please let me know.

krismach commented 6 months ago

Thanks @colinaut, 1.7.27 fixes the bug I've experienced in 25.

kevinmu17 commented 6 months ago

No, v1.7.27 is still broken when using name="message[body]"

with this example i hope you get the same errors:

    <form x-data x-validate @submit="$validate.submit">
        <div>
            <input type="email" name="testing" required>
            <div class="error-msg text-sm mt-2 text-red-600">
                E-mailadres is required
            </div>
        </div>
        <button type="submit">send</button>
    </form>

<!-- THIS FORM BREAKS -->
    <form x-data x-validate @submit="$validate.submit">
        <div>
            <input type="email" name="testing[body]" required>
            <div class="error-msg text-sm mt-2 text-red-600">
                E-mailadres is required
            </div>
        </div>
        <button type="submit">send</button>
    </form>
colinaut commented 6 months ago

@kevinmu17 There must be something else going on with your code as it works for me. Here is a Codepen showing no errors. https://codepen.io/colinaut/pen/GRLJrYY

kingbiscit commented 6 months ago

Im seeing this issue with checkbox groups.

colinaut commented 6 months ago

@kingbiscit do you have a code example?

andrewdisley commented 6 months ago

@colinaut the issue with checkboxes is is visible on the example or via the npm run serve when running 1.7.27

After submitting the form with empty values then selecting Cat or Dog in the Favorite Animals section. The error 'you must pick at least one animal' remains and there is a console error Uncaught TypeError: Cannot read properties of null (reading 'setAttribute')

colinaut commented 6 months ago

@andrewdisley @kingbiscit Thanks I figured out the issue (was introduced in changes I made with other recent releases) and it has been fixed in the latest release 1.7.28

kevinmu17 commented 6 months ago

Awesome, that fixed my issues to! all working great again :) thanks for the efforts!

kingbiscit commented 6 months ago

Fantastic i'll do an update on our app later today. Thank you.