Attachment 87939 Details for Bug 22657 – Bug 22657: Remove JavaScript from OPAC suggestion validation of required fields

[patch] Bug 22657: Remove JavaScript from OPAC suggestion validation of required fields

Bug-22657-Remove-JavaScript-from-OPAC-suggestion-v.patch (text/plain), 18.14 KB, created by Bin Wen on 2019-04-12 19:55:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Bin Wen

Created: 2019-04-12 19:55:19 UTC

Size: 18.14 KB

patch

obsolete

>From a21190e7b3972435e2a02edfb7f4d6c21cd7b9c2 Mon Sep 17 00:00:00 2001
>From: Owen Leonard <oleonard@myacpl.org>
>Date: Wed, 20 Mar 2019 18:58:02 +0000
>Subject: [PATCH] Bug 22657: Remove JavaScript from OPAC suggestion validation
> of required fields
>
>This patch modifies the OPAC suggestion form so that it doesn't require
>JavaScript for form validation. This change doesn't alter the behavior
>of the form for users but does fit with the general goal of progressive
>enhancement for the OPAC.
>
>The patch adds "required" labels to required fields to better identify
>required fields.
>
>To test, apply the patch and make sure there are required fields
>specified in the OPACSuggestionMandatoryFields system preference.
>
> - Log in to the OPAC and go to Your purchase suggestions -> New
>   purchase suggestion.
> - Verify that the fields specified in OPACSuggestionMandatoryFields are
>   highlighted and marked "required."
> - Verify that you can't submit the form without filling out these
>   fields.
>
>Signed-off-by: Bin Wen <bin.wen@inlibro.com>
>---
> .../bootstrap/en/modules/opac-suggestions.tt  | 162 ++++++++++++++----
> opac/opac-suggestions.pl                      |  11 +-
> 2 files changed, 134 insertions(+), 39 deletions(-)
>
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt
>index f560e642b2..5ce43f2718 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt
>@@ -43,42 +43,145 @@
>                             <form action="/cgi-bin/koha/opac-suggestions.pl" method="post" id="add_suggestion_form">
>                                 <fieldset class="rows">
>                                     <ol>
>-                                        <li><label for="title">Title:</label><input type="text" id="title" name="title" class="span6" maxlength="255" /></li>
>-                                        <li><label for="author">Author:</label><input type="text" id="author" name="author" class="span6" maxlength="80" /></li>
>+                                        <li>
>+                                            [% IF ( title_required ) %]
>+                                                <label for="title" class="required">Title:</label>
>+                                                <input type="text" id="title" name="title" class="span6" maxlength="255" required="required" />
>+                                                <span class="required">Required</span>
>+                                            [% ELSE %]
>+                                                <label for="title">Title:</label>
>+                                                <input type="text" id="title" name="title" class="span6" maxlength="255" />
>+                                            [% END %]
>+                                        </li>
>+                                        <li>
>+                                            [% IF ( author_required ) %]
>+                                                <label for="author" class="required">Author:</label>
>+                                                <input type="text" id="author" name="author" class="span6" maxlength="80" required="required" />
>+                                                <span class="required">Required</span>
>+                                            [% ELSE %]
>+                                                <label for="author">Author:</label>
>+                                                <input type="text" id="author" name="author" class="span6" maxlength="80" />
>+                                            [% END %]
>+                                        </li>
>                                         <li>
>                                             <div title="Copyright or publication year, for example: 2016">
>-                                            <label for="copyrightdate">Copyright date:</label><input type="text" id="copyrightdate" name="copyrightdate" pattern="[12]\d{3}" size="4" maxlength="4" />
>+                                                [% IF ( copyrightdate_required ) %]
>+                                                    <label for="copyrightdate" class="required">Copyright date:</label>
>+                                                    <input type="text" id="copyrightdate" name="copyrightdate" pattern="[12]\d{3}" size="4" maxlength="4" required="required" />
>+                                                <span class="required">Required</span>
>+                                                [% ELSE %]
>+                                                    <label for="copyrightdate">Copyright date:</label>
>+                                                    <input type="text" id="copyrightdate" name="copyrightdate" pattern="[12]\d{3}" size="4" maxlength="4" />
>+                                                [% END %]
>                                             </div>
>                                         </li>
>-                                        <li><label for="isbn">Standard number (ISBN, ISSN or other):</label><input type="text" id="isbn" name="isbn"  maxlength="80" /></li>
>-                                        <li><label for="publishercode">Publisher:</label><input type="text" id="publishercode" name="publishercode" class="span6" maxlength="80" /></li>
>-                                        <li><label for="collectiontitle">Collection title:</label><input type="text" id="collectiontitle" name="collectiontitle" class="span6" maxlength="80" /></li>
>-                                        <li><label for="place">Publication place:</label><input type="text" id="place" name="place"  maxlength="80" /></li>
>-                                        <li id="opac-suggestion-quantity"><label for="quantity">Quantity:</label><input type="text" id="quantity" name="quantity"  maxlength="4" size="4" /></li>
>-                                        <li><label for="itemtype">Item type:</label>
>-                                            [% PROCESS 'av-build-dropbox.inc' name="itemtype", category="SUGGEST_FORMAT", size = 20 %]
>+                                        <li>
>+                                            [% IF ( isbn_required ) %]
>+                                                <label for="isbn" class="required">Standard number (ISBN, ISSN or other):</label>
>+                                                <input type="text" id="isbn" name="isbn"  maxlength="80" required="required" />
>+                                                <span class="required">Required</span>
>+                                            [% ELSE %]
>+                                                <label for="isbn">Standard number (ISBN, ISSN or other):</label>
>+                                                <input type="text" id="isbn" name="isbn"  maxlength="80" />
>+                                            [% END %]
>+                                        </li>
>+                                        <li>
>+                                            [% IF ( publishercode_required ) %]
>+                                                <label for="publishercode" class="required">Publisher:</label>
>+                                                <input type="text" id="publishercode" name="publishercode" class="span6" maxlength="80" required="required" />
>+                                                <span class="required">Required</span>
>+                                            [% ELSE %]
>+                                                <label for="publishercode">Publisher:</label>
>+                                                <input type="text" id="publishercode" name="publishercode" class="span6" maxlength="80" />
>+                                            [% END %]
>+                                        </li>
>+                                        <li>
>+                                            [% IF ( collectiontitle_required ) %]
>+                                                <label for="collectiontitle" class="required">Collection title:</label>
>+                                                <input type="text" id="collectiontitle" name="collectiontitle" class="span6" maxlength="80" required="required" />
>+                                                <span class="required">Required</span>
>+                                            [% ELSE %]
>+                                                <label for="collectiontitle">Collection title:</label>
>+                                                <input type="text" id="collectiontitle" name="collectiontitle" class="span6" maxlength="80" />
>+                                            [% END %]
>+                                        </li>
>+                                        <li>
>+                                            [% IF ( place_required ) %]
>+                                                <label for="place" class="required">Publication place:</label>
>+                                                <input type="text" id="place" name="place" required="required" maxlength="80" />
>+                                                <span class="required">Required</span>
>+                                            [% ELSE %]
>+                                                <label for="place">Publication place:</label>
>+                                                <input type="text" id="place" name="place"  maxlength="80" />
>+                                            [% END %]
>+                                        </li>
>+                                        <li id="opac-suggestion-quantity">
>+                                            [% IF ( quantity_required ) %]
>+                                                <label for="quantity" class="required">Quantity:</label>
>+                                                <input type="text" id="quantity" name="quantity" required="required" maxlength="4" size="4" />
>+                                                <span class="required">Required</span>
>+                                            [% ELSE %]
>+                                                <label for="quantity">Quantity:</label>
>+                                                <input type="text" id="quantity" name="quantity"  maxlength="4" size="4" />
>+                                            [% END %]
>+                                        </li>
>+                                        <li>
>+                                            [% IF ( itemtype_required ) %]
>+                                                <label for="itemtype" class="required">Item type:</label>
>+                                                [% PROCESS 'av-build-dropbox.inc' name="itemtype", category="SUGGEST_FORMAT", size = 20 %]
>+                                                <span class="required">Required</span>
>+                                            [% ELSE %]
>+                                                <label for="itemtype">Item type:</label>
>+                                                [% PROCESS 'av-build-dropbox.inc' name="itemtype", category="SUGGEST_FORMAT", size = 20 %]
>+                                            [% END %]
>                                         </li>
>                                         [% IF branchcode %]
>-                                            <li><label for="branch">Library:</label>
>-                                                <select name="branchcode" id="branch">
>-                                                    [% PROCESS options_for_libraries libraries => Branches.all( selected => branchcode ) %]
>-                                                </select>
>+                                            <li>
>+                                                [% IF ( branchcode_required ) %]
>+                                                    <label for="branch" class="required">Library:</label>
>+                                                    <select name="branchcode" id="branch" required="required">
>+                                                        [% PROCESS options_for_libraries libraries => Branches.all( selected => branchcode ) %]
>+                                                    </select>
>+                                                    <span class="required">Required</span>
>+                                                [% ELSE %]
>+                                                    <label for="branch">Library:</label>
>+                                                    <select name="branchcode" id="branch">
>+                                                        [% PROCESS options_for_libraries libraries => Branches.all( selected => branchcode ) %]
>+                                                    </select>
>+                                                [% END %]
>                                             </li>
>                                         [% END %]
>                                         [% IF ( patron_reason_loop ) %]
>                                             <li>
>-                                                <label for="patronreason">Reason for suggestion: </label>
>-                                                <select name="patronreason" id="patronreason">
>-                                                    <option value="">-- Choose --</option>
>-                                                    [% FOREACH patron_reason_loo IN patron_reason_loop %]
>-                                                        <option value="[% patron_reason_loo.authorised_value | html %]">[% patron_reason_loo.lib | html %]</option>
>-                                                    [% END %]
>-                                                </select>
>+                                                [% IF ( patronreason_required ) %]
>+                                                    <label for="patronreason" class="required">Reason for suggestion: </label>
>+                                                    <select name="patronreason" id="patronreason" required="required">
>+                                                        <option value="">-- Choose --</option>
>+                                                        [% FOREACH patron_reason_loo IN patron_reason_loop %]
>+                                                            <option value="[% patron_reason_loo.authorised_value | html %]">[% patron_reason_loo.lib | html %]</option>
>+                                                        [% END %]
>+                                                    </select>
>+                                                    <span class="required">Required</span>
>+                                                [% ELSE %]
>+                                                    <label for="patronreason">Reason for suggestion: </label>
>+                                                    <select name="patronreason" id="patronreason">
>+                                                        <option value="">-- Choose --</option>
>+                                                        [% FOREACH patron_reason_loo IN patron_reason_loop %]
>+                                                            <option value="[% patron_reason_loo.authorised_value | html %]">[% patron_reason_loo.lib | html %]</option>
>+                                                        [% END %]
>+                                                    </select>
>+                                                [% END %]
>                                             </li>
>                                         [% END %]
>                                         <li>
>-                                            <label for="note">Notes:</label>
>-                                            <textarea name="note" id="note" rows="5" cols="40"></textarea>
>+                                            [% IF ( note_required ) %]
>+                                                <label for="note" class="required">Notes:</label>
>+                                                <textarea name="note" id="note" rows="5" cols="40" required="required"></textarea>
>+                                                <span class="required">Required</span>
>+                                            [% ELSE %]
>+                                                <label for="note">Notes:</label>
>+                                                <textarea name="note" id="note" rows="5" cols="40"></textarea>
>+                                            [% END %]
>                                         </li>
> 
>                                         
>@@ -339,19 +442,6 @@
>           return true;
>         });
>         [% END %]
>-        [% IF ( op_add && mandatoryfields ) %]
>-        {
>-            var FldsRequired = [[% mandatoryfields | $raw %]];
>-            for (var i = 0; i < FldsRequired.length; i++) {
>-                var rq_input = $('#' + FldsRequired[i]);
>-                if (rq_input.length != 1) continue;
>-                $(rq_input).attr("required", "required");
>-                var rq_label = $("label[for=" + rq_input.attr("id") + "]");
>-                if (rq_label.length != 1) continue;
>-                $(rq_label).addClass('required');
>-            }
>-        }
>-        [% END %]
>     });
> //]]>
> </script>
>diff --git a/opac/opac-suggestions.pl b/opac/opac-suggestions.pl
>index cf562fb97e..9c2b86162c 100755
>--- a/opac/opac-suggestions.pl
>+++ b/opac/opac-suggestions.pl
>@@ -208,13 +208,19 @@ if ( C4::Context->preference("AllowPurchaseSuggestionBranchChoice") ) {
>     $template->param( branchcode => $branchcode );
> }
> 
>-my $mandatoryfields = '';
>+my @mandatoryfields;
>+
> {
>     last unless ($op eq 'add');
>     my $fldsreq_sp = C4::Context->preference("OPACSuggestionMandatoryFields") || 'title';
>-    $mandatoryfields = join(', ', (map { '"'.$_.'"'; } sort split(/\s*\,\s*/, $fldsreq_sp)));
>+    @mandatoryfields = split( ",", $fldsreq_sp );
>+    foreach (@mandatoryfields) {
>+        $template->param( $_."_required" => 1);
>+    }
> }
> 
>+
>+
> $template->param(
>     %$suggestion,
>     suggestions_loop      => $suggestions_loop,
>@@ -224,7 +230,6 @@ $template->param(
>     messages              => \@messages,
>     suggestionsview       => 1,
>     suggested_by_anyone   => $suggested_by_anyone,
>-    mandatoryfields       => $mandatoryfields,
>     patrons_pending_suggestions_count => $patrons_pending_suggestions_count,
> );
> 
>-- 
>2.17.1

Actions: View | Diff | Splinter Review

Attachments on bug 22657: 87526 | 87939 | 88132 | 89504 | 89520 | 89525 | 89526