Created attachment 13790 [details] Screenshot Duplicate patron - prepopulated fields cleared How to reproduce: 1) Go to a patron's detail page 2) Click "Duplicate" 3) On the screen, click in one of the prepopulated fields, e.g. Surname or City Result: The field is cleared. Expected: Cursor is set without clearing the field. Is this a bug or is it by design?
This is by design. I think the idea is that you don't click those fields if the data is correct. If the data is incorrect you want to replace it, so clearing is helpful. It's easy to see the downside: If you use the keyboard to navigate through the form you'll end up clearing all the existing data. I don't know that I've ever used the duplicate feature in production, but I would think that downside is pretty significant, not to mention being an accessibility problem for users who can't use a mouse. Is this a feature we really need to keep?
It feels like unexpected behaviour to me - might be more irritating than helpful.
Created attachment 33460 [details] [review] Bug 9177 - Duplicate patron: Prepopulated fields ar cleared when setting cursor When adding a duplicated patron record some fields are prepopulated from the original patron record. Focusing your cursor on these fields automatically clears them. This works well if you assume that the user only focuses on fields which should be changed. This works badly if you consider that a user might want to tab between all fields. I think the disadvantages outweigh the minor benefit of not having to delete the contents of a field. This patch removes the clearing action. To test, apply the patch and duplicate an existing patron record. Tabbing through the entry fields should not result in the contents of those fields being cleared.
Created attachment 33722 [details] [review] Bug 9177 - Duplicate patron: Prepopulated fields ar cleared when setting cursor When adding a duplicated patron record some fields are prepopulated from the original patron record. Focusing your cursor on these fields automatically clears them. This works well if you assume that the user only focuses on fields which should be changed. This works badly if you consider that a user might want to tab between all fields. I think the disadvantages outweigh the minor benefit of not having to delete the contents of a field. This patch removes the clearing action. TEST PLAN --------- 1) Log in to staff client 2) Patrons 3) Find an existing patron 4) Click on the link in the Name column 5) Click on the Duplicate button 6) Click on fields with data in them. -- they will clear, which is more problematic than correcting the entry. 7) On the line "Home > Patrons > {patron name} > Duplicate Adult Patron" click the patron name to go back. 8) Click on the Duplicate button 9) Tab through the fields -- the fields should remain unchanged. 10) Apply the patch 11) On the line "Home > Patrons > {patron name} > Duplicate Adult Patron" click the patron name to go back. 12) Click on the Duplicate button 13) Navigate through the fields -- tabbing nor clicking should clear any fields. 14) Run koha qa tests REBASED
Created attachment 33723 [details] [review] [SIGNED OFF] Bug 9177 - Duplicate patron: Prepopulated fields ar cleared when setting cursor When adding a duplicated patron record some fields are prepopulated from the original patron record. Focusing your cursor on these fields automatically clears them. This works well if you assume that the user only focuses on fields which should be changed. This works badly if you consider that a user might want to tab between all fields. I think the disadvantages outweigh the minor benefit of not having to delete the contents of a field. This patch removes the clearing action. TEST PLAN --------- 1) Log in to staff client 2) Patrons 3) Find an existing patron 4) Click on the link in the Name column 5) Click on the Duplicate button 6) Click on fields with data in them. -- they will clear, which is more problematic than correcting the entry. 7) On the line "Home > Patrons > {patron name} > Duplicate Adult Patron" click the patron name to go back. 8) Click on the Duplicate button 9) Tab through the fields -- the fields should remain unchanged, but sadly they clear. 10) Apply the patch 11) On the line "Home > Patrons > {patron name} > Duplicate Adult Patron" click the patron name to go back. 12) Click on the Duplicate button 13) Navigate through the fields -- tabbing nor clicking should clear any fields. 14) Run koha qa tests NOTE: Had to rebase and tweak, but looks good. Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
As a side comment, I don't think all the fields that should be cleared when you duplicate are cleared, but that is not within the scope of this bug.
Created attachment 33727 [details] [review] [PASSED QA] Bug 9177 - Duplicate patron: Prepopulated fields ar cleared when setting cursor When adding a duplicated patron record some fields are prepopulated from the original patron record. Focusing your cursor on these fields automatically clears them. This works well if you assume that the user only focuses on fields which should be changed. This works badly if you consider that a user might want to tab between all fields. I think the disadvantages outweigh the minor benefit of not having to delete the contents of a field. This patch removes the clearing action. TEST PLAN --------- 1) Log in to staff client 2) Patrons 3) Find an existing patron 4) Click on the link in the Name column 5) Click on the Duplicate button 6) Click on fields with data in them. -- they will clear, which is more problematic than correcting the entry. 7) On the line "Home > Patrons > {patron name} > Duplicate Adult Patron" click the patron name to go back. 8) Click on the Duplicate button 9) Tab through the fields -- the fields should remain unchanged, but sadly they clear. 10) Apply the patch 11) On the line "Home > Patrons > {patron name} > Duplicate Adult Patron" click the patron name to go back. 12) Click on the Duplicate button 13) Navigate through the fields -- tabbing nor clicking should clear any fields. 14) Run koha qa tests NOTE: Had to rebase and tweak, but looks good. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Less surprises, improves accessibility by enabling to tab through the form without losing data.
(In reply to M. Tompsett from comment #6) > As a side comment, I don't think all the fields that should be cleared when > you duplicate are cleared, but that is not within the scope of this bug. That sounds like Bug 7878 - When duplicating a patron too many fields carrying over - a fix for which was just pushed.
Patch doesn't apply anymore, presumably because of the changes made by the QA follow-up to Bug 7878.
Created attachment 33753 [details] [review] [REBASED] Bug 9177 - Duplicate patron: Prepopulated fields ar cleared when setting cursor When adding a duplicated patron record some fields are prepopulated from the original patron record. Focusing your cursor on these fields automatically clears them. This works well if you assume that the user only focuses on fields which should be changed. This works badly if you consider that a user might want to tab between all fields. I think the disadvantages outweigh the minor benefit of not having to delete the contents of a field. This patch removes the clearing action. TEST PLAN --------- 1) Log in to staff client 2) Patrons 3) Find an existing patron 4) Click on the link in the Name column 5) Click on the Duplicate button 6) Click on fields with data in them. -- they will clear, which is more problematic than correcting the entry. 7) On the line "Home > Patrons > {patron name} > Duplicate Adult Patron" click the patron name to go back. 8) Click on the Duplicate button 9) Tab through the fields -- the fields should remain unchanged, but sadly they clear. 10) Apply the patch 11) On the line "Home > Patrons > {patron name} > Duplicate Adult Patron" click the patron name to go back. 12) Click on the Duplicate button 13) Navigate through the fields -- tabbing nor clicking should clear any fields. 14) Run koha qa tests NOTE: Had to rebase again. Yes, bug 7878 interfered. However, it deals with comment #6, which Owen appropriately noted. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Less surprises, improves accessibility by enabling to tab through the form without losing data. NOTE: Set back to Passed QA from Patch doesn't apply.
Patch pushed to master. Thanks Owen!