From efeed28f7f58dffe0a307616a5f565321b55e8c0 Mon Sep 17 00:00:00 2001 From: Owen Leonard Date: Wed, 10 Feb 2021 19:43:25 +0000 Subject: [PATCH] Bug 27668: Improve validation of patron entry form in the OPAC This patch improves the way we handle required fields in the patron entry/update form in the OPAC. Instead of doing multiple checks for each field using mandatory.defined(), the template now loops over a list of fields and sets a "required" variable for any which are required. Then, for each form field, the "required" variable is used to set classes on labels, input fields, and the "required" text hints. The class on form fields acts as a hook for the jQuery validator plugin. The class on the text hints allows us to hide the text hint using CSS, eliminating the necessity of using a template conditional. The patch also adds the missing validator-strings include which enables translation of the jQuery validator plugin's validation error messages. To test, apply the patch and rebuild the OPAC CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client). - Go to Administration -> System preferences and enable PatronSelfRegistration. - Select multiple fields to be required using the PatronSelfRegistrationBorrowerMandatoryField preference. - In the OPAC, start the process of registering for an account. - The fields you designated as mandatory should each have a "Required" label under them. - Try to submit the form without entering any data. The required fields should now be highlighted in red and have another label, "This field is required." - Fill in the required fields and submit the form. It should submit correctly. - Modify the PatronSelfRegistrationBorrowerMandatoryField preference and select all fields as mandatory. Confirm that all fields in the patron entry form work correctly. - Test that form validation works correctly when modifying a logged-in patron's existing account. Signed-off-by: Katrin Fischer Signed-off-by: Martin Renvoize Signed-off-by: Jonathan Druart --- .../opac-tmpl/bootstrap/css/src/opac.scss | 40 +- .../en/includes/validator-strings.inc | 24 + .../bootstrap/en/modules/opac-memberentry.tt | 481 ++++++------------ 3 files changed, 211 insertions(+), 334 deletions(-) create mode 100644 koha-tmpl/opac-tmpl/bootstrap/en/includes/validator-strings.inc diff --git a/koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss b/koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss index 8f411ae789..37c843a918 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss +++ b/koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss @@ -624,8 +624,24 @@ th { } } +div { + &.required_label { + display: none; + + &.required { + color: #C00; + display: block; + font-size: 95%; + margin-left: 10rem; + margin-top: 3px; + } + } +} + .required { - color: #C00; + &.valid { + color: #000; + } } @@ -686,12 +702,21 @@ fieldset { .label { float: left; font-weight: bold; - margin-right: 1em; + margin-right: 1rem; text-align: right; - width: 9em; + width: 9rem; } label { + &.error { + color: #C00; + float: none; + font-style: italic; + font-weight: normal; + margin-left: 1rem; + text-align: left; + width: auto; + } &.lradio { float: none; margin: inherit; @@ -744,7 +769,7 @@ fieldset { .hint { display: block; - margin-left: 11em; + margin-left: 10rem; } } @@ -784,7 +809,7 @@ div { font-weight: bold; margin-right: 1em; text-align: left; - width: 9em; + width: 9rem; } } @@ -2246,14 +2271,11 @@ nav { #memberentry-form { input.error { border-color: #C00; - box-shadow: 0 1px 1px #C00 inset, 0 0 8px #C00; - color: #F00; outline: 0 none; &:focus { border-color: #C00; - box-shadow: 0 1px 1px #C00 inset, 0 0 8px #C00; - color: #F00; + box-shadow: inset 0 1px 1px rgba(0, 0, 0, .075), 0 0 8px rgba(204, 0, 0, .6); outline: 0 none; } diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/validator-strings.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/validator-strings.inc new file mode 100644 index 0000000000..0600f0a672 --- /dev/null +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/validator-strings.inc @@ -0,0 +1,24 @@ + + + diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt index fc46fedda5..580a780767 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt @@ -10,14 +10,10 @@ [% BLOCK streetnumber %] [% UNLESS hidden.defined('streetnumber') %]
  • - [% IF mandatory.defined('streetnumber') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('streetnumber') %]Required[% END %] + +
    Required
    • [% END %] [% END %] @@ -191,6 +187,12 @@
    + [% FOREACH field = ['streetnumber' 'cardnumber' 'branchcode' 'categorycode' 'title' 'surname' 'firstname' 'dateofbirth' 'initials' 'othernames' 'address' 'address2' 'city' 'state' 'zipcode' 'country' 'phone' 'phonepro' 'mobile' 'email' 'emailpro' 'fax' 'B_address' 'B_address2' 'B_city' 'B_state' 'B_zipcode' 'B_country' 'B_phone' 'B_email' 'contactnote' 'altcontactsurname' 'altcontactfirstname' 'altcontactaddress1' 'altcontactaddress2' 'altcontactaddress3' 'altcontactstate' 'altcontactzipcode' 'altcontactcountry' 'altcontactphone' 'password' ] %] + [% IF mandatory.defined( field ) %] + [% SET required.$field = 'required' %] + [% END %] + [% END %] + [%# Following on one line for translatability %] [% UNLESS ( hidden.defined('cardnumber') || ( !borrower && Koha.Preference('autoMemberNum') ) ) && hidden.defined('dateexpiry') && hidden.defined('branchcode') && hidden.defined('categorycode') %]
    @@ -201,29 +203,27 @@
      [% UNLESS hidden.defined('cardnumber') || ( !borrower && Koha.Preference('autoMemberNum') ) %]
    1. - [% IF mandatory.defined('cardnumber') %] - - [% ELSE %] - - [% END %] + + + [% IF borrower && !(cardnumber_wrong_length || cardnumber_already_exists) %] [% borrower.cardnumber | html %] [% ELSE %] [% IF minlength_cardnumber == maxlength_cardnumber %] - - [% IF ( mandatory.defined('cardnumber') ) %]Required[% END %] + +
      Required
      Card number must be exactly [% minlength_cardnumber | html %] characters.
      [% ELSIF minlength_cardnumber && maxlength_cardnumber %] - - [% IF ( mandatory.defined('cardnumber') ) %]Required[% END %] + +
      Required
      Card number must be between [% minlength_cardnumber | html %] and [% maxlength_cardnumber | html %] characters.
      [% ELSIF maxlength_cardnumber %] - - [% IF ( mandatory.defined('cardnumber') ) %]Required[% END %] + +
      Required
      Card number can be up to [% maxlength_cardnumber | html %] characters.
      [% ELSE %] - - [% IF ( mandatory.defined('cardnumber') ) %]Required[% END %] + +
      Required
      There is no minimum or maximum character length.
      [% END %] [% END %] @@ -240,13 +240,9 @@ [% UNLESS hidden.defined('branchcode') %]
    2. [% IF ( libraries.size > 1 ) %] - [% IF mandatory.defined('branchcode') %] - - [% ELSE %] - - [% END %] + - [% FOREACH l IN libraries %] [% IF l.branchcode == borrower.branchcode %] @@ -255,6 +251,7 @@ [% END %] [% END %] +
      Required
      [% ELSE %] Home library: [% FOREACH l IN libraries %] @@ -267,14 +264,14 @@ [% UNLESS hidden.defined('categorycode') %]
    3. -
      4. [% END %] @@ -302,13 +300,9 @@
        [% UNLESS hidden.defined('title') || !Koha.Preference('BorrowersTitles') %]
      1. - [% IF mandatory.defined('title') %] - - [% ELSE %] - - [% END %] + - [% FOREACH mt IN Koha.Preference('BorrowersTitles').split('\|') %] [% IF mt == borrower.title %] @@ -318,44 +312,33 @@ [% END %] [% END %] +
        Required
        2. [% END %] [% UNLESS hidden.defined('surname') %]
      2. - [% IF mandatory.defined('surname') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('surname') %]Required[% END %] + +
        Required
        3. [% END %] [% UNLESS hidden.defined('firstname') %]
      3. - [% IF mandatory.defined('firstname') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('firstname') %]Required[% END %] + +
        Required
        4. [% END %] [% UNLESS hidden.defined('dateofbirth') %]
      4. - [% IF mandatory.defined('dateofbirth') %] - - [% ELSE %] - - [% END %] + - + [% UNLESS action == 'edit' && !OPACPatronDetails %] [% UNLESS ( mandatory.defined('dateofbirth') ) %] @@ -363,33 +346,25 @@ [% END %] [% END %] - [% IF mandatory.defined('dateofbirth') %]Required[% END %] +
        Required
        5. [% END %] [% UNLESS hidden.defined('initials') %]
      5. - [% IF mandatory.defined('initials') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('initials') %]Required[% END %] + +
        Required
        6. [% END %] [% UNLESS hidden.defined('othernames') %]
      6. - [% IF mandatory.defined('othernames') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('othernames') %]Required[% END %] + +
        Required
        7. [% END %] @@ -449,14 +424,10 @@ [% UNLESS hidden.defined('address') %]
      7. - [% IF mandatory.defined('address') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('address') %]Required[% END %] + +
        Required
        8. [% END %] @@ -464,66 +435,46 @@ [% UNLESS hidden.defined('address2') %]
      8. - [% IF mandatory.defined('address2') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('address2') %]Required[% END %] + +
        Required
        9. [% END %] [% UNLESS hidden.defined('city') %]
      9. - [% IF mandatory.defined('city') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('city') %]Required[% END %] + +
        Required
        10. [% END %] [% UNLESS hidden.defined('state') %]
      10. - [% IF mandatory.defined('state') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('state') %]Required[% END %] + +
        Required
        11. [% END %] [% UNLESS hidden.defined('zipcode') %]
      11. - [% IF mandatory.defined('zipcode') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('zipcode') %]Required[% END %] + +
        Required
        12. [% END %] [% UNLESS hidden.defined('country') %]
      12. - [% IF mandatory.defined('country') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('country') %]Required[% END %] + +
        Required
        13. [% END %] @@ -543,92 +494,64 @@
          [% UNLESS hidden.defined('phone') %]
        1. - [% IF mandatory.defined('phone') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('phone') %]Required[% END %] + +
          Required
          2. [% END %] [% UNLESS hidden.defined('phonepro') %]
        2. - [% IF mandatory.defined('phonepro') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('phonepro') %]Required[% END %] + +
          Required
          3. [% END %] [% UNLESS hidden.defined('mobile') %]
        3. - [% IF mandatory.defined('mobile') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('mobile') %]Required[% END %] + +
          Required
          4. [% END %] [% UNLESS hidden.defined('email') %]
        4. - [% IF mandatory.defined('email') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('email') %]Required[% END %] + +
          Required
          5. [% IF action != 'edit' and Koha.Preference('PatronSelfRegistrationConfirmEmail') %]
        5. - [% IF mandatory.defined('email') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('email') %]Required[% END %] + +
          Required
          6. [% END %] [% END %] [% UNLESS hidden.defined('emailpro') %]
        6. - [% IF mandatory.defined('emailpro') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('emailpro') %]Required[% END %] + +
          Required
          7. [% END %] [% UNLESS hidden.defined('fax') %]
        7. - [% IF mandatory.defined('fax') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('fax') %]Required[% END %] + +
          Required
          8. [% END %]
        @@ -647,118 +570,82 @@
          [% UNLESS hidden.defined('B_address') %]
        1. - [% IF mandatory.defined('B_address') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('B_address') %]Required[% END %] + +
          Required
          2. [% END %] [% UNLESS hidden.defined('B_address2') %]
        2. - [% IF mandatory.defined('B_address2') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('B_address2') %]Required[% END %] + +
          Required
          3. [% END %] [% UNLESS hidden.defined('B_city') %]
        3. - [% IF mandatory.defined('B_city') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('B_city') %]Required[% END %] + +
          Required
          4. [% END %] [% UNLESS hidden.defined('B_state') %]
        4. - [% IF mandatory.defined('B_state') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('B_state') %]Required[% END %] + +
          Required
          5. [% END %] [% UNLESS hidden.defined('B_zipcode') %]
        5. - [% IF mandatory.defined('B_zipcode') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('B_zipcode') %]Required[% END %] + +
          Required
          6. [% END %] [% UNLESS hidden.defined('B_country') %]
        6. - [% IF mandatory.defined('B_country') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('B_country') %]Required[% END %] + +
          Required
          7. [% END %] [% UNLESS hidden.defined('B_phone') %]
        7. - [% IF mandatory.defined('B_phone') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('B_phone') %]Required[% END %] + +
          Required
          8. [% END %] [% UNLESS hidden.defined('B_email') %]
        8. - [% IF mandatory.defined('B_email') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('B_email') %]Required[% END %] + +
          Required
          9. [% END %] [% UNLESS hidden.defined('contactnote') %]
        9. - [% IF mandatory.defined('contactnote') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('contactnote') %]Required[% END %] + +
          Required
          10. [% END %] @@ -778,118 +665,82 @@
            [% UNLESS hidden.defined('altcontactsurname') %]
          1. - [% IF mandatory.defined('altcontactsurname') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('altcontactsurname') %]Required[% END %] + +
            Required
            2. [% END %] [% UNLESS hidden.defined('altcontactfirstname') %]
          2. - [% IF mandatory.defined('altcontactfirstname') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('altcontactfirstname') %]Required[% END %] + +
            Required
            3. [% END %] [% UNLESS hidden.defined('altcontactaddress1') %]
          3. - [% IF mandatory.defined('altcontactaddress1') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('altcontactaddress1') %]Required[% END %] + +
            Required
            4. [% END %] [% UNLESS hidden.defined('altcontactaddress2') %]
          4. - [% IF mandatory.defined('altcontactaddress2') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('altcontactaddress2') %]Required[% END %] + +
            Required
            5. [% END %] [% UNLESS hidden.defined('altcontactaddress3') %]
          5. - [% IF mandatory.defined('altcontactaddress3') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('altcontactaddress3') %]Required[% END %] + +
            Required
            6. [% END %] [% UNLESS hidden.defined('altcontactstate') %]
          6. - [% IF mandatory.defined('altcontactstate') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('altcontactstate') %]Required[% END %] + +
            Required
            7. [% END %] [% UNLESS hidden.defined('altcontactzipcode') %]
          7. - [% IF mandatory.defined('altcontactzipcode') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('altcontactzipcode') %]Required[% END %] + +
            Required
            8. [% END %] [% UNLESS hidden.defined('altcontactcountry') %]
          8. - [% IF mandatory.defined('altcontactcountry') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('altcontactcountry') %]Required[% END %] + +
            Required
            9. [% END %] [% UNLESS hidden.defined('altcontactphone') %]
          9. - [% IF mandatory.defined('altcontactphone') %] - - [% ELSE %] - - [% END %] + - - [% IF mandatory.defined('altcontactphone') %]Required[% END %] + +
            Required
            10. [% END %]
          @@ -918,27 +769,16 @@ [% END %]
    - [% IF mandatory.defined('password') %] -
      -
    1. - - Required -
      2. -
    2. - - Required -
      3. -
    - [% ELSE %] -
      -
    1. - -
      2. -
    2. - -
      3. -
    - [% END %] +
      +
    1. + +
      Required
      +
      2. +
    2. + +
      Required
      +
      3. +
    @@ -987,7 +827,7 @@ [% END %] [% IF pa.type.mandatory %] - Required +
    Required
    [% END %] Clear [% IF ( pa.type.repeatable ) %] @@ -1017,7 +857,7 @@ GDPR consent
    1. - I agree with your processing of my personal data as outlined in the privacy policy. Required + I agree with your processing of my personal data as outlined in the privacy policy.
      Required
    @@ -1036,6 +876,7 @@ +
    Required
    Please type the following characters into the preceding box: [% captcha | html %] @@ -1075,6 +916,7 @@ [% INCLUDE 'opac-bottom.inc' %] [% BLOCK jsinclude %] + [% INCLUDE 'validator-strings.inc' %] [% Asset.js("lib/jquery/plugins/jquery.validate.min.js") | $raw %] [% INCLUDE 'calendar.inc' %] @@ -1105,16 +947,10 @@ email: true }, borrower_password: { - [% IF mandatory.defined('password') %] - required: true, - [% END %] password_strong: true, password_no_spaces: true }, borrower_password2: { - [% IF mandatory.defined('password') %] - required: true, - [% END %] password_match: true }, captcha: { @@ -1129,18 +965,13 @@ form.beenSubmitted = true; form.submit(); } - }, - errorPlacement: function(error, element) { - offset = element.offset(); - error.insertAfter(element) - error.addClass('error'); // add a class to the wrapper - error.css('position', 'absolute'); - error.css('left', offset.left + element.outerWidth() + 10); - error.css('top', offset.top); - error.css('width', 'auto'); } }); + $("input.required,select.required,textarea.required").rules("add", { + required: true + }); + [% IF patron.guarantor_relationships && !Koha.Preference('OPACPrivacy') %] [% IF Koha.Preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') %] @@ -1254,7 +1085,7 @@ $('select#borrower_categorycode').change(setPwdMessage); }); [% END %] - //]]> + [% PROCESS 'password_check.inc' new_password => 'borrower_password', category_selector => '#borrower_categorycode', RequireStrongPassword => patron ? patron.category.effective_require_strong_password : defaultCategory.effective_require_strong_password, minPasswordLength => patron ? patron.category.effective_min_password_length : defaultCategory.effective_min_password_length %] -- 2.39.5