|
Description
Katrin Fischer
2013-05-17 08:04:49 UTC
16 sounds quite restrictive - could it be made longer too? Here are some steps we could take, individually or in combination: 1. Add a "maxlenth" attribute to the input field. 2. Add some JavaScript validation to warn the user when the limit is reached. 3. Increase the length which can be stored. If we increase the possible length of a card number what should it be? 50? 100? I think I'd like to see a combination of JavaScript warning and maxlength. 16 seems like a reasonable size for a cardnumber, but not sure if there are use cases where a longer number would be needed. I don't think this is valid anymore - I am unable to type anything once the card number has reached 16 characters in the input box. Please confirm if this is still valid? I can confirm this bug by doing this: 1. Bring up the new patron form. 2. Along with any other required fields, enter '23467890758428435' as the barcode (17 digits). 3. Save the patron record. The barcode will be saved as '2346789075842843' (16 digits, the last digit missing). Oh okay I see! So this only (In reply to Owen Leonard from comment #5) > I can confirm this bug by doing this: > > 1. Bring up the new patron form. > 2. Along with any other required fields, enter '23467890758428435' as the > barcode (17 digits). > 3. Save the patron record. The barcode will be saved as '2346789075842843' > (16 digits, the last digit missing). I don't get your problem, when I paste that number it removes the 5 straight away from the text box. (In reply to Aleisha Amohia from comment #6) > when I paste that number it removes the 5 straight > away from the text box. Okay, my mistake. When I tested it I could have sworn the field let me put in a longer number. Either way, if the user isn't carefully watching the number go in they will not be aware that something is wrong. Imagine scanning a barcode -- will the user visually confirm that the number matches what they scanned? This isn't something which is necessarily badly broken, it's just something we can do better with. Created attachment 62440 [details] [review] Bug 10267: Show error message if user tries to enter too many characters for cardnumber This patch displays a message if the user tries to enter more than the max number of characters for a cardnumber. To test: 1) Apply patch and go to Patrons -> New patron 2) Scroll down to Card number 3) Put in any characters. Notice that when you have entered the max number of characters, you are unable to type any more. 4) If you try to type again, the error message will show up. 5) if you backspace some characters, the message should disappear Sponsored-by: Catalyst IT Created attachment 62442 [details] [review] Bug 10267: Show error message if user tries to enter too many characters for cardnumber This patch displays a message if the user tries to enter more than the max number of characters for a cardnumber. To test: 1) Apply patch and go to Patrons -> New patron 2) Scroll down to Card number 3) Put in any characters. Notice that when you have entered the max number of characters, you are unable to type any more. 4) If you try to type again, the error message will show up. 5) if you backspace some characters, the message should disappear Sponsored-by: Catalyst IT Followed test plan, works as expected Signed-off-by: Marc Véron <veron@veron.ch> Oh, I forgot to sitsch status... Aleisha, the last case is wrong, maxlength_cardnumber is evaluated false and the message will be "Card number must not be more than characters". That should not be possible (because C4::Members::get_cardnumber_length returns max=16), but the code does not make sense as it is. Moreover you should not use the keypress event but change, that way a paste or delete in the input will update the message as well. Created attachment 62806 [details] [review] Bug 10267: Show error message if user tries to enter too many characters for cardnumber This patch displays a message if the user tries to enter more than the max number of characters for a cardnumber. To test: 1) Apply patch and go to Patrons -> New patron 2) Scroll down to Card number 3) Put in any characters. Notice that when you have entered the max number of characters, you are unable to type any more. 4) Click out of the text field (so it loses focus), the error message will show up. 5) if you backspace some characters and click out of the text field again, the message should disappear Sponsored-by: Catalyst IT Followed test plan, works as expected Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 62934 [details] [review] [SIGNED OFF] Bug 10267: Show error message if user tries to enter too many characters for cardnumber This patch displays a message if the user tries to enter more than the max number of characters for a cardnumber. To test: 1) Apply patch and go to Patrons -> New patron 2) Scroll down to Card number 3) Put in any characters. Notice that when you have entered the max number of characters, you are unable to type any more. 4) Click out of the text field (so it loses focus), the error message will show up. 5) if you backspace some characters and click out of the text field again, the message should disappear Sponsored-by: Catalyst IT Followed test plan, works as expected Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Hmm I do not really understand why we need this message. You cannot type more than 16 chars or paste them. There is a hint that it should be between 1 and 16. Currently, the message shows up at 16 chars. This makes me think that I entered too much chars, but I did not. I think that if we want to do something on this matter, we need to search for another angle. But I am not convinced that we should; the hint is clear to me. We could allow more than 16 chars and then mark it red or something ? (In reply to Marcel de Rooy from comment #14) > Hmm I do not really understand why we need this message. > > You cannot type more than 16 chars or paste them. > There is a hint that it should be between 1 and 16. > Currently, the message shows up at 16 chars. This makes me think that I > entered too much chars, but I did not. > > I think that if we want to do something on this matter, we need to search > for another angle. But I am not convinced that we should; the hint is clear > to me. We could allow more than 16 chars and then mark it red or something ? Owen explains the benefit of a patch like this in Comment 7. It may not be broken and seems very obvious to us, but the point is to make things as clear as possible for the user. Related bug, signed off: Bug 13178 - cardnumber field length is too short It expands the card number length to 32. Tested on BibLibre's sandbox #2 The patch applies as described in Comment #13. However, now there is a new patch for bug 13178 and it is not taken in count in the current bug, I don't know if I'm supposed to sign it :/ Comment on attachment 62934 [details] [review] [SIGNED OFF] Bug 10267: Show error message if user tries to enter too many characters for cardnumber Review of attachment 62934 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ +621,4 @@ > [% END %] > [% IF minlength_cardnumber == maxlength_cardnumber %] > <input type="text" id="cardnumber" name="cardnumber" size="20" value="[% cardnumber | html %]" minlength="[% minlength_cardnumber %]" maxlength="[% maxlength_cardnumber %]" /> > + [% IF mandatorycardnumber and not autoMemberNum %]<span class="required">Required</span>[% END %]<span id="cn_max" class="required">Card number must not be more than [% maxlength_cardnumber %] characters.</span> It is impossible to enter more than maxlength_cardnumber characters because the maxlength attribute of the input line makes it impossible. I believe bug 13178, providing flexibility of the max size based on the schema which should be regenerated as needed to match the table's field size, makes this bug obsolete. It's impossible to enter them, but scanning from an ID or c&p from another system you might not know that it has been automatically cut off. This was just thought to provide some help for this case. I am signing off. Created attachment 65657 [details] [review] [SIGNED OFF] Bug 10267: Show error message if user tries to enter too many characters for cardnumber This patch displays a message if the user tries to enter more than the max number of characters for a cardnumber. To test: 1) Apply patch and go to Patrons -> New patron 2) Scroll down to Card number 3) Put in any characters. Notice that when you have entered the max number of characters, you are unable to type any more. 4) Click out of the text field (so it loses focus), the error message will show up. 5) if you backspace some characters and click out of the text field again, the message should disappear Sponsored-by: Catalyst IT Followed test plan, works as expected Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> If we increase the size of borrowers.cardnumber to 32 (bug 13178), do we still need this? I haven't tested the other bug, but this is not dependent on the size I think. They still might conflict. On master I cannot input more than the max number of characters for cardnumber. Is this patch still necessary? Created attachment 66732 [details] [review] Bug 10267: Show error message if user tries to enter too many characters for cardnumber This patch displays a message if the user tries to enter more than the max number of characters for a cardnumber. To test: 1) Apply patch and go to Patrons -> New patron 2) Scroll down to Card number 3) Put in any characters. Notice that when you have entered the max number of characters, you are unable to type any more. 4) Click out of the text field (so it loses focus), the error message will show up. 5) if you backspace some characters and click out of the text field again, the message should disappear Sponsored-by: Catalyst IT Followed test plan, works as expected Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> The problem is not input - try and copy a longer string in. You won't notice that it has been cut off - with the patch you get a note. With CardnumberLength=16, if I enter a 16 chars length string, I get the warning. It should display only if > 16, right? (In reply to Jonathan Druart from comment #26) > With CardnumberLength=16, if I enter a 16 chars length string, I get the > warning. > It should display only if > 16, right? Not quite - you can't actually input more than 16 characters so no warning would show if we made that change, which is the problem already. Having a warning at 16 characters (rather than > 16 characters) just lets the user know that there is a limit in place and that they've reached the max. Does that make sense? Created attachment 68503 [details] [review] Bug 10267: Display the message only if the user enters too many characters It seems better to display the warning if the user tries to enter too many characters in the input. Test plan: With max=16 1. Copy/paste a string with 15, 16 and 17 characters 2. Enter a cardnumber of 15, 16, 17 characters The warning should be displayed only the input overflows Aleisha, can you take a look at this try please? (In reply to Jonathan Druart from comment #29) > Aleisha, can you take a look at this try please? Ah yes I see how your change works, I like it and it makes sense. Will sign off on it. Created attachment 68504 [details] [review] Bug 10267: Display the message only if the user enters too many characters It seems better to display the warning if the user tries to enter too many characters in the input. Test plan: With max=16 1. Copy/paste a string with 15, 16 and 17 characters 2. Enter a cardnumber of 15, 16, 17 characters The warning should be displayed only the input overflows Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Created attachment 68724 [details] [review] Bug 10267: Show error message if user tries to enter too many characters for cardnumber This patch displays a message if the user tries to enter more than the max number of characters for a cardnumber. To test: 1) Apply patch and go to Patrons -> New patron 2) Scroll down to Card number 3) Put in any characters. Notice that when you have entered the max number of characters, you are unable to type any more. 4) Click out of the text field (so it loses focus), the error message will show up. 5) if you backspace some characters and click out of the text field again, the message should disappear Sponsored-by: Catalyst IT Followed test plan, works as expected Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 68725 [details] [review] Bug 10267: Display the message only if the user enters too many characters It seems better to display the warning if the user tries to enter too many characters in the input. Test plan: With max=16 1. Copy/paste a string with 15, 16 and 17 characters 2. Enter a cardnumber of 15, 16, 17 characters The warning should be displayed only the input overflows Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed to master for 17.11, thanks to everybody involved! |