Bug 21362 - Advanced MARC Editor - Rancor - Tab navigation not working in fixed fields
Summary: Advanced MARC Editor - Rancor - Tab navigation not working in fixed fields
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Ere Maijala
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-09-16 18:49 UTC by Ere Maijala
Modified: 2018-10-24 09:13 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 21362: Advanced editor: Fix tab navigation in fixed fields (5.31 KB, patch)
2018-09-16 18:57 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21362: Advanced editor: Fix tab navigation in fixed fields (5.38 KB, patch)
2018-09-16 20:37 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 21362: Advanced editor: Fix tab navigation in fixed fields (5.46 KB, patch)
2018-09-21 06:59 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21362: Advanced editor: Fix tab navigation in fixed fields (5.58 KB, patch)
2018-09-21 10:34 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21362: Advanced editor: Fix tab navigation in fixed fields (5.63 KB, patch)
2018-09-21 19:02 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 21362: Advanced editor: Fix tab navigation in fixed fields (5.79 KB, patch)
2018-09-24 12:00 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 21362: Advanced editor: Fix tab navigation in fixed fields (5.84 KB, patch)
2018-09-25 10:04 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 21362: Advanced editor: Fix tab navigation in fixed fields (5.90 KB, patch)
2018-09-27 12:44 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ere Maijala 2018-09-16 18:49:29 UTC

    
Comment 1 Ere Maijala 2018-09-16 18:50:49 UTC
It's possible to move between fields with tab/shift-tab but when there's a dropdown menu in a control field, it's not possible to move past it.
Comment 2 Ere Maijala 2018-09-16 18:57:23 UTC
Created attachment 78990 [details] [review]
Bug 21362: Advanced editor: Fix tab navigation in fixed fields

Also minor tweaks to how the fields look.
Comment 3 Brendan Gallagher 2018-09-16 20:37:51 UTC
Created attachment 79000 [details] [review]
Bug 21362: Advanced editor: Fix tab navigation in fixed fields

Also minor tweaks to how the fields look.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 4 Jonathan Druart 2018-09-18 13:14:01 UTC
Ere, can you add a test plan please? I have linked a control field to AV but I do not see the dropdown list.

Also I think the following change is wrong:
-                        if ( ( this.nodeName.toLowerCase() == 'select' && e.which == 9 ) || e.ctrlKey ) {
+                        if ( e.ctrlKey && 0 ) {
Comment 5 Katrin Fischer 2018-09-20 20:23:09 UTC
Maybe he meant the default plugins? They have a lot of pull downs.
Comment 6 Ere Maijala 2018-09-21 06:59:36 UTC
Created attachment 79191 [details] [review]
Bug 21362: Advanced editor: Fix tab navigation in fixed fields

Also minor tweaks to how the fields look.

Test plan:

1. Try moving from field to field and inside control fields with tab/shift-tab.
2. Make sure focus doesn't get stuck in any position.
Comment 7 Ere Maijala 2018-09-21 07:00:10 UTC
Thanks, Jonathan, that was indeed a leftover. Patch fixed and test plan added.
Comment 8 Ere Maijala 2018-09-21 10:34:25 UTC
Created attachment 79210 [details] [review]
Bug 21362: Advanced editor: Fix tab navigation in fixed fields

Also minor tweaks to how the fields look.

Test plan:

1. Try moving from field to field and inside control fields with tab/shift-tab.
2. Make sure focus doesn't get stuck in any position.
Comment 9 Ere Maijala 2018-09-21 10:35:16 UTC
Fixed patch really attached.
Comment 10 Michal Denar 2018-09-21 19:02:08 UTC
Created attachment 79248 [details] [review]
Bug 21362: Advanced editor: Fix tab navigation in fixed fields

Also minor tweaks to how the fields look.

Test plan:

1. Try moving from field to field and inside control fields with tab/shift-tab.
2. Make sure focus doesn't get stuck in any position.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 11 Jonathan Druart 2018-09-21 19:44:35 UTC
Seetings > Show helpers for fixed and coded fields!
Comment 12 Jonathan Druart 2018-09-21 20:08:11 UTC
So basically you are reverting the changes made by

commit 3b2d66cec899dd3b33f41041bafb785bda7e1e31
    Bug 11559: (followup) fix first-character deletion, small usability issues

    This should fix the first-character deletion bug reported by Nick, and also:

      * Fix subfield deletion
      * Attempted fix for special keys on dropdowns
      * Allow deleting empty fields
      * Fix bug with composed characters

Which was a fix for bug 11559 comment 145.

I found at least one regression:
"-Ctrl+D when dropdown is selected bookmarks page instead of adding a new field"

Maybe we should be more specific (instead of removing conditions) and catch alt+tab and alt+shift+tab (?)
Comment 13 Ere Maijala 2018-09-24 11:17:24 UTC
Thanks again, Jonathan. I'll check and revise once more.
Comment 14 Ere Maijala 2018-09-24 12:00:09 UTC
Created attachment 79299 [details] [review]
Bug 21362: Advanced editor: Fix tab navigation in fixed fields

Also minor tweaks to how the fields look.

Test plan:

1. Try moving from field to field and inside control fields with tab/shift-tab.
2. Make sure focus doesn't get stuck in any position.
Comment 15 Ere Maijala 2018-09-24 12:01:54 UTC
Latest version keeps the old condition apart from bailing out from select with tab.
Comment 16 Michal Denar 2018-09-24 12:44:43 UTC
Hi Ere,
It works. It stuck inside coded fields like 008 or on fields with lists like 942. 

Do You able to fix it in this or we make new bug?

Short video: http://recordit.co/lYsQzCnbY4


Thank You

Mike
Comment 17 Ere Maijala 2018-09-24 13:16:24 UTC
Michal, your video shows exactly the issue I'm trying to fix with this patch. Do you still encounter it with the patch applied?
Comment 18 Michal Denar 2018-09-24 13:23:49 UTC
Hi,
Video shows behaviour after patch. But I'm newbie, maybe some issue between keyboard and chair :-)

Mike
Comment 19 Ere Maijala 2018-09-25 06:19:53 UTC
Which browser and operating system?
Comment 20 Michal Denar 2018-09-25 07:17:28 UTC
Hi,

Firefox 62.0.2, MacOS

I'try Chrome too.


Mike
Comment 21 Ere Maijala 2018-09-25 08:13:27 UTC
Same OS and browser work for me with the patch applied. Can you verify again that you have the patch and the browser doesn't cache old widget.js?
Comment 22 Michal Denar 2018-09-25 10:04:46 UTC
Created attachment 79360 [details] [review]
Bug 21362: Advanced editor: Fix tab navigation in fixed fields

Also minor tweaks to how the fields look.

Test plan:

1. Try moving from field to field and inside control fields with tab/shift-tab.
2. Make sure focus doesn't get stuck in any position.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 23 Katrin Fischer 2018-09-27 12:44:43 UTC
Created attachment 79498 [details] [review]
Bug 21362: Advanced editor: Fix tab navigation in fixed fields

Also minor tweaks to how the fields look.

Test plan:

1. Try moving from field to field and inside control fields with tab/shift-tab.
2. Make sure focus doesn't get stuck in any position.

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Nick Clemens 2018-09-28 19:57:37 UTC
Awesome work all!

Pushed to master for 18.11
Comment 25 Martin Renvoize 2018-10-03 09:35:04 UTC
Pushed to 18.05.x for 18.05.05
Comment 26 Fridolin SOMERS 2018-10-10 10:06:42 UTC
Pushed to 17.11.x for 17.11.11