DavyJonesLocker / client_side_validations-simple_form

Simple Form plugin for ClientSideValidations
MIT License
254 stars 102 forks source link

isValid throwing false to valid inputs and error messages incomplete #104

Closed isaporto closed 3 years ago

isaporto commented 4 years ago

I am doing a devise registration multi-step using stimulus and following part of https://github.com/DavyJonesLocker/client_side_validations-simple_form/wiki/Validations-with-simple_form-Twitter-Bootstrap-integration but I did some changes because wasn't working properly:

let valid = true;
$('input:visible, select:visible').each(function() {
  const settings = window[this.form.id].ClientSideValidations.settings;
  if (!$(this).isValid(settings.validators)) {
    valid = false
  }
});
if (valid) {
  //code to go to next step
  if (e.currentTarget.id === "nextBtn") this.index++;
}

Steps to reproduce

First bug (isValid throwing false): Get redirect to registration page by a link. Then I tried to trigger validation unfocusing from input or clicking in next btn. Works normally if I refresh the page. Second bug (Error message incomplete): unfocus inputs to trigger validation throught gem

Expected behavior

First bug: To email input focus normally. Unfocus at input should trigger validations to show up errors messages in case have any. Click at next button should too trigger validations and shows errors messages or, in case of all inputs are valid, go to the next tab. Second bug: The errors messages include also the input type like "Email can't be blank"

Actual behavior

First bug: Unfocus and click on next btn didn't trigger anything. I tried debug and it seems like $(this).isValid(settings.validators) always throw false, even if I fill all inputs with valid values. It didn't shows up any errors messages too. Second bug: Error message shows up like "can't be blank".

System configuration*

Rails version: 6.0.3.4

Ruby version: 2.6.6

Client Side Validations version: 17.1.0

Client Side Validations Simple Form version: 11.1.0

Bootstrap version (if used): 4.5.3

Code snippet from your model of the validations*

devise validation and validates :role, presence: true

Relevant part of application.js

require('@client-side-validations/client-side-validations') require('@client-side-validations/simple-form/dist/simple-form.bootstrap4') it's after turbolinks.start()

The whole form code from your template

<%= simple_form_for(resource, as: resource_name, url: registration_path(resource_name), validate: true, data: { controller: "form" }) do |f| %>
    <%= f.error_notification %>

    <div class="tab" data-target="form.tab">
      <%= f.input :email,
                  required: true,
                  autofocus: true,
                  input_html: { autocomplete: "email" },
                  validate: true %>
      <%= f.input :password,
                  required: true,
                  hint: ("#{@minimum_password_length} characters minimum" if @minimum_password_length),
                  input_html: { autocomplete: "new-password" },
                  validate: true %>
      <%= f.input :password_confirmation,
                  required: true,
                  input_html: { autocomplete: "new-password" } %>
    </div>

    <div class="tab" data-target="form.tab">
      <%= f.input :role, collection: %w[pj manager],
                  validate: true %>
    </div>

    <div class="form-actions">
      <button class="btn-orange" type="button" id="prevBtn" data-action="form#nextPrev" data-target="form.prevBtn">Previous</button>
      <button class="btn-orange" type="button" id="nextBtn" data-action="form#nextPrev" data-target="form.nextBtn">Next</button>
    </div>
  <% end %>

The resulting HTML*

    <form class="simple_form new_user" id="new_user" novalidate="novalidate" data-controller="form" data-client-side-validations="{&quot;html_settings&quot;:{&quot;type&quot;:&quot;SimpleForm::FormBuilder&quot;,&quot;error_class&quot;:&quot;invalid-feedback&quot;,&quot;error_tag&quot;:&quot;div&quot;,&quot;wrapper_error_class&quot;:&quot;form-group-invalid&quot;,&quot;wrapper_tag&quot;:&quot;div&quot;,&quot;wrapper_class&quot;:&quot;form-group&quot;,&quot;wrapper&quot;:&quot;vertical_form&quot;},&quot;number_format&quot;:{&quot;separator&quot;:&quot;.&quot;,&quot;delimiter&quot;:&quot;,&quot;},&quot;validators&quot;:{&quot;user[email]&quot;:{&quot;presence&quot;:[{&quot;message&quot;:&quot;can't be blank&quot;}],&quot;uniqueness&quot;:[{&quot;message&quot;:&quot;has already been taken&quot;,&quot;case_sensitive&quot;:true,&quot;allow_blank&quot;:true}],&quot;format&quot;:[{&quot;message&quot;:&quot;is invalid&quot;,&quot;with&quot;:{&quot;source&quot;:&quot;^[^@\\s]+@[^@\\s]+$&quot;,&quot;options&quot;:&quot;&quot;},&quot;allow_blank&quot;:true}]},&quot;user[password]&quot;:{&quot;presence&quot;:[{&quot;message&quot;:&quot;can't be blank&quot;}],&quot;confirmation&quot;:[{&quot;message&quot;:&quot;doesn't match Password&quot;,&quot;case_sensitive&quot;:true}],&quot;length&quot;:[{&quot;messages&quot;:{&quot;minimum&quot;:&quot;is too short (minimum is 6 characters)&quot;,&quot;maximum&quot;:&quot;is too long (maximum is 128 characters)&quot;},&quot;allow_blank&quot;:true,&quot;minimum&quot;:6,&quot;maximum&quot;:128}]},&quot;user[role]&quot;:{&quot;presence&quot;:[{&quot;message&quot;:&quot;can't be blank&quot;}]}}}" action="/users" accept-charset="UTF-8" method="post"><input type="hidden" name="authenticity_token" value="ccfkjL7QIt6kcFNVkn0Emu5bdXnt+DXVieim/YbPBXHiywdNiPE1u5qjyohRyK3MplNwyhJtG4T+I9ts9WGasQ==">

  <div class="tab current-tab" data-target="form.tab">
      <div class="form-group email required user_email"><label class="email required" for="user_email">Email <abbr title="required">*</abbr></label><input class="form-control string email required" autocomplete="email" autofocus="autofocus" required="required" aria-required="true" type="email" value="" name="user[email]" id="user_email"></div>
      <div class="form-group password required user_password"><label class="password required" for="user_password">Password <abbr title="required">*</abbr></label><input class="form-control password required" autocomplete="new-password" required="required" aria-required="true" type="password" name="user[password]" id="user_password"><small class="form-text text-muted">6 characters minimum</small></div>
      <div class="form-group password required user_password_confirmation"><label class="password required" for="user_password_confirmation">Password confirmation <abbr title="required">*</abbr></label><input class="form-control password required" autocomplete="new-password" required="required" aria-required="true" type="password" name="user[password_confirmation]" id="user_password_confirmation"></div>
    </div>

    <div class="tab" data-target="form.tab">
      <div class="form-group select required user_role"><label class="select required" for="user_role">Role <abbr title="required">*</abbr></label>
            <select class="form-control select required" name="user[role]" id="user_role">
            <option value=""></option>
            <option value="pj">pj</option>
            <option value="manager">manager</option>
            </select>
      </div>
    </div>
    <div class="form-actions">
      <button class="btn-orange" type="button" id="prevBtn" data-action="form#nextPrev" data-target="form.prevBtn" style="display: none;">Previous</button>
      <button class="btn-orange" type="button" id="nextBtn" data-action="form#nextPrev" data-target="form.nextBtn">Next</button>
    </div>
  </form>

Browser's development console output*

Additional JavaScript Libraries*

Using JS framework Stimulus

Repository demostrating the issue

Debugging CSV issues is a time consuming task. If you want to speed up things, please provide a link to a repository showing the issue.


* Failure to include this requirement may result in the issue being closed.

tagliala commented 4 years ago

Hi!

Thanks for being part of the CSV Community.

I am doing a devise registration multi-step using stimulus and following part of https://github.com/DavyJonesLocker/client_side_validations-simple_form/wiki/Validations-with-simple_form-Twitter-Bootstrap-integration

I've deleted that wiki page, it is from 2012! 😅

CSV Simple Form supports bootstrap 3 and 4 out of the box

I have a multistep form using CSV which looks like yours:

https://github.com/diowa/icare/blob/705b63e3a4ab03810d2a768699df9b00a7011c38/app/javascript/src/javascripts/maps-new.js#L99-L106

Is it possible to share a repository to reproduce this issue with the minimum amount of code?

Debugging CSV is really time consuming, and there are stimulus and multistep forms involved, so I don't know where to start

In my test playground, with the snippet from your HTML, I can see validations, so the problem should be somewhere else

image

isaporto commented 4 years ago

Here it is https://github.com/isaporto/multiple-step-test I created a new repository with few routes and gems very simple and with the same error. The wiki it's rlly old but I couldn't find a new one up-to-date sooo meh

tagliala commented 4 years ago

@isaporto thanks.

Much easier to replicate and debug.

The issue depends on dynamically added fields that are activated when showCurrentTab javascript is executed

Ref: https://github.com/DavyJonesLocker/client_side_validations/wiki/Validating-dynamically-added-forms

diff --git a/app/javascript/controllers/form_controller.js b/app/javascript/controllers/form_controller.js
index 2370d75..efb4668 100644
--- a/app/javascript/controllers/form_controller.js
+++ b/app/javascript/controllers/form_controller.js
@@ -11,7 +11,7 @@ export default class extends Controller {
   nextPrev(e) {
     // if the form is valid then go to the next tab else don't
     let valid = true;
-    $('input:visible, select:visible').each(function() {
+    $('[data-validate]:input:visible').each(function() {
       const settings = window[this.form.id].ClientSideValidations.settings;
       if (!$(this).isValid(settings.validators)) {
         valid = false
@@ -39,6 +39,7 @@ export default class extends Controller {
     this.index = index; // index who communicates with nextPrev
     this.tabTargets.forEach((el, i) => {
       el.classList.toggle("current-tab", this.index == i) // displayig the speciefied tab of the form
+      $(el).closest('form[data-client-side-validations]').validate()
     })
     // Fixing Prev/Next buttons
     index === 0 ? this.prevBtnTarget.style.display = "none" : this.prevBtnTarget.style.display = "flex"

This should work

isaporto commented 4 years ago

Gosh, It was so simple! Thanks for the quick assistance!

isaporto commented 3 years ago

Sorry, bother you but I actually forgot about the fields name missing in the error messages

tagliala commented 3 years ago

Could you please clarify?

You want email can't be blank as an error message?

isaporto commented 3 years ago

yes, I know that the error message is can't be blank only, but, when I submit, simple form the message is Email can't be blank and I think it's prettier

tagliala commented 3 years ago

Sorry, this is not possible out of the box

Feel free to request it as a feature in the main repo at https://github.com/DavyJonesLocker/client_side_validations-simple, but I will label as help-wanted because I do not have resources to implement it

You can customize error rendering by following this guide: https://github.com/DavyJonesLocker/client_side_validations#customize-error-rendering

For your specific use case, I think this will work:

import $ from 'jquery'
import ClientSideValidations from '@client-side-validations/client-side-validations'

ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = {
  add: function (element, settings, message) {
    this.wrapper(settings.wrapper).add.call(this, element, settings, message)
  },
  remove: function (element, settings) {
    this.wrapper(settings.wrapper).remove.call(this, element, settings)
  },
  wrapper: function (name) {
    return this.wrappers[name] || this.wrappers.default
  },

  wrappers: {
    default: {
      add (element, settings, message) {
        const wrapperElement = element.parent()
        let errorElement = wrapperElement.find(settings.error_tag + '.invalid-feedback')

        if (!errorElement.length) {
          errorElement = $('<' + settings.error_tag + '>', { class: 'invalid-feedback', text: message })
          wrapperElement.append(errorElement)
        }

        wrapperElement.addClass(settings.wrapper_error_class)
        element.addClass('is-invalid')
        errorElement.text(`${wrapperElement.find('label').clone().children().remove().end().text()} ${message}`)
      },

      remove (element, settings) {
        const wrapperElement = element.parent()
        const errorElement = wrapperElement.find(settings.error_tag + '.invalid-feedback')

        wrapperElement.removeClass(settings.wrapper_error_class)
        element.removeClass('is-invalid')
        errorElement.remove()
      }
    }
  }
}

Note the changed line from the original file at https://github.com/DavyJonesLocker/client_side_validations-simple_form/blob/master/src/main.bootstrap4.js:

errorElement.text(`${wrapperElement.find('label').clone().children().remove().end().text()} ${message}`)