Bug 7882 - Add ability to move and reorder fields and subfields in MARC editor
Summary: Add ability to move and reorder fields and subfields in MARC editor
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Maryse Simard
QA Contact: Jonathan Druart
URL:
Keywords:
: 2124 11073 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-04-02 21:55 UTC by Melia Meggs
Modified: 2020-11-17 20:06 UTC (History)
28 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
This feature allows to change the sequence of tags and subfields in the cataloguing editor usind drag & drop.
Version(s) released in:
20.05.00, 19.11.05, 19.05.10


Attachments
Adds jQueryUI sortable features to fields. Converts list items from div's to ul's Added Image files to be used for Drag and drop in frameworks added CSS changes for images (9.59 KB, patch)
2012-11-01 21:46 UTC, Elliott Davis
Details | Diff | Splinter Review
Rebase for master -- DO NOT USE (23.22 KB, patch)
2012-11-20 14:57 UTC, Elliott Davis
Details | Diff | Splinter Review
Bug 7882: Add ability to sort fields and subfields (24.52 KB, patch)
2012-11-29 21:22 UTC, Elliott Davis
Details | Diff | Splinter Review
Bug 7882: Add ability to sort fields and subfields (24.43 KB, patch)
2012-12-19 21:13 UTC, Elliott Davis
Details | Diff | Splinter Review
Bug 7882 [fixed] (7.90 KB, patch)
2013-01-29 06:24 UTC, Elliott Davis
Details | Diff | Splinter Review
Bug 7882 - move or reorder 6xx and 7xx fields (8.32 KB, patch)
2013-05-08 13:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7882 - move or reorder 6xx and 7xx fields (8.37 KB, patch)
2013-05-08 13:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7882 - move or reorder 6xx and 7xx fields - Followup - Remove placeholder highlighting (1.29 KB, patch)
2013-05-08 13:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7882 - move or reorder 6xx and 7xx fields - Followup - Remove placeholder highlighting (1.51 KB, patch)
2013-05-08 13:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7882 - move or reorder 6xx and 7xx fields (7.07 KB, patch)
2013-11-08 16:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7882 - move or reorder 6xx and 7xx fields - Followup - Remove placeholder highlighting (1.51 KB, patch)
2013-11-08 16:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7882 - move or reorder 6xx and 7xx fields (6.98 KB, patch)
2013-12-13 15:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7882 - move or reorder 6xx and 7xx fields (7.04 KB, patch)
2013-12-13 16:36 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 7882: Inputs should be editable (1.27 KB, patch)
2014-01-07 13:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7882 - Add ability to move and reorder fields in MARC editor (7.06 KB, patch)
2014-01-13 12:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7882: Inputs should be editable (1.31 KB, patch)
2014-01-13 12:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7882: Add ability to move and reorder fields in MARC editor (23.24 KB, patch)
2019-10-23 19:36 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 7882: Add ability to move and reorder fields in MARC editor (23.39 KB, patch)
2019-10-29 14:52 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 7882: Add ability to move and reorder fields in MARC editor (23.58 KB, patch)
2019-12-16 14:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7882: (follow-up) Update sorting interaction (2.97 KB, patch)
2019-12-16 19:13 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7882: (follow-up) Only show sortable icon for sortable tags (3.34 KB, patch)
2019-12-17 15:23 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 7882: (follow-up) Update sorting interaction (3.01 KB, patch)
2019-12-20 22:27 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 7882: (follow-up) Only show sortable icon for sortable tags (3.38 KB, patch)
2019-12-20 22:27 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 7882: Add ability to move and reorder fields in MARC editor (23.65 KB, patch)
2019-12-26 11:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7882: (follow-up) Update sorting interaction (3.09 KB, patch)
2019-12-26 11:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7882: (follow-up) Only show sortable icon for sortable tags (3.46 KB, patch)
2019-12-26 11:34 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Melia Meggs 2012-04-02 21:55:08 UTC
Would like to be able to move or reorder 6xx and 7xx fields.  The only way to change the order of 700s is to copy or cut /paste. We would like to be able to easily move production people on DVDs to below the actors.  Or change the order of 6xx fields to more accurately describe the subject of an item.
Comment 1 Elliott Davis 2012-11-01 21:46:49 UTC Comment hidden (obsolete)
Comment 2 Elliott Davis 2012-11-15 12:47:12 UTC
oops forgot to set the correct flag
Comment 3 Elliott Davis 2012-11-20 14:57:13 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2012-11-21 13:55:41 UTC
The reordering works great, but the clone and delete tag links appear to be broken.
Comment 5 Nicole C. Engard 2012-11-26 13:51:48 UTC
I wanted to add myself to the CC list, but couldn't without setting a patch complexity - please update the complexity if I guessed wrong.

Nicole
Comment 6 Elliott Davis 2012-11-29 21:22:33 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2012-12-04 10:30:55 UTC
Hi Elliott,

Some remarks:

1/ Could you provide a patch with yours modifications and 1 patch for the indentation please ? It is not easy at all to read the patch.

2/ I just have fields on the first tab, the others tabs are empty.

3/ The title for the cursor_split image is wrong ("move up")

4/ the following if statement is not graceful:
     if(flag){
         return StrAlert;
     } else {
         return flag;
     }
It could be replaced with if ( flag ) return StrAlert; return 0;

Marked as Failed QA.
Comment 8 Elliott Davis 2012-12-19 21:13:47 UTC Comment hidden (obsolete)
Comment 9 Elliott Davis 2012-12-20 15:03:32 UTC
(In reply to comment #7)
> Hi Elliott,
> 
> Some remarks:
> 
> 1/ Could you provide a patch with yours modifications and 1 patch for the
> indentation please ? It is not easy at all to read the patch.
> 
> 2/ I just have fields on the first tab, the others tabs are empty.
> 
I have corrected this issue

> 3/ The title for the cursor_split image is wrong ("move up")
> 
I corrected this to say move
> 4/ the following if statement is not graceful:
>      if(flag){
>          return StrAlert;
>      } else {
>          return flag;
>      }
> It could be replaced with if ( flag ) return StrAlert; return 0;
> 
This was added with the tab fix.  It appears that the file had tabs in it before and when I replaced them with spaces I got more diff than I bargained for.
> Marked as Failed QA.

I don't suppose there is any way to let the changes in whitespace slide.  I think to correct them I'd have to re-write the patch.
Comment 10 Jonathan Druart 2012-12-21 12:52:09 UTC
(In reply to comment #9)
> > 2/ I just have fields on the first tab, the others tabs are empty.
> > 
> I have corrected this issue
> 
> > 3/ The title for the cursor_split image is wrong ("move up")
> > 
> I corrected this to say move

Thanks!


> This was added with the tab fix.  It appears that the file had tabs in it
> before and when I replaced them with spaces I got more diff than I bargained
> for.

You mean your editor automatically replaces tab with 4 spaces when editing a file?

> I don't suppose there is any way to let the changes in whitespace slide.  I
> think to correct them I'd have to re-write the patch.

Yes I know it is not easy for this patch. But try to think about that for your next patch :)

There is no SO for your patch so I change the status to "Needs Signoff".
Comment 11 Elliott Davis 2012-12-21 14:28:13 UTC
not automatically, but before I found the git whitespace fix from rangi's blog I had it replace tabs with spaces.  Thanks for letting it slide this once and sorry for setting it to the wrong status.  I assumed that since it was you (Jonathan) it was going through QA.  Thats what I get for not looking at the backlog ;)

Elliott Davis
Comment 12 Owen Leonard 2012-12-28 17:01:20 UTC
This appears to be working at first glance, but I see that there is a problem somewhere with the markup, causing the language-switcher menu not to "stick" to the bottom of the screen. This is probably caused by an unclosed <div> somewhere.

I hate to say it, but I really think this patch needs to be submitted without all the whitespace changes. addbiblio.pl is a complex page, and not being able to carefully review the relevant changes makes me nervous.
Comment 13 Elliott Davis 2013-01-29 06:24:56 UTC Comment hidden (obsolete)
Comment 14 Bernardo Gonzalez Kriegel 2013-01-29 22:57:00 UTC
Work as described. Tags and subtags could be reordered.
But the result is a bit ugly, and there is too much indentation on subtags.
That's cosmetic.

The trouble I found is that can't collapse or expand tags.
Comment 15 Elliott Davis 2013-02-26 13:12:00 UTC
(In reply to comment #14)

> The trouble I found is that can't collapse or expand tags.

Was this existing functionality?  I wasn't aware that you could collapse or expand tags before.
Comment 16 Nicole C. Engard 2013-02-26 14:07:22 UTC
Elliott,

Clicking on the description of the tag let you expand or collapse it.

Nicole
Comment 17 Frederic Durand 2013-03-20 13:25:29 UTC
I ve tried it on a sandbox, it did not work, here s the error message :
Applying: Bug 7882 [fixed] Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/css/addbiblio.css Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
Comment 18 Jonathan Druart 2013-03-20 14:17:21 UTC
(In reply to comment #17)
> I ve tried it on a sandbox, it did not work, here s the error message :
> Applying: Bug 7882 [fixed] Using index info to reconstruct a base tree...
> Falling back to patching base and 3-way merge... Auto-merging
> koha-tmpl/intranet-tmpl/prog/en/css/addbiblio.css Auto-merging
> koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt

It is not blocker for testing. The patch was correctly applied.
Comment 19 Frederic Durand 2013-03-20 16:16:12 UTC
I confirm that expand/collapse is off.
When you want to modify you can t do it just clicking on it, you need to click the tag or subtag title or on the left of it
Remark/question : when you want to drag and move your field you got a yellow area, i suppose it s done to show you where you are going to move? In this case it misses its goal cause it tends to stay on the top.
Hope i m not too blurry in my explanations :)
Comment 20 Kyle M Hall 2013-05-08 13:39:53 UTC Comment hidden (obsolete)
Comment 21 Kyle M Hall 2013-05-08 13:40:49 UTC Comment hidden (obsolete)
Comment 22 Kyle M Hall 2013-05-08 13:40:58 UTC Comment hidden (obsolete)
Comment 23 Kyle M Hall 2013-05-08 13:42:25 UTC Comment hidden (obsolete)
Comment 24 Bernardo Gonzalez Kriegel 2013-05-08 17:50:26 UTC
Kyle,
I could move subfields but not fields
(ej a <-> b, but not 610 <-> 611)

Is that the expected behavior?
Comment 25 Bernardo Gonzalez Kriegel 2013-05-10 00:18:45 UTC
(In reply to comment #24)
> Kyle,
> I could move subfields but not fields
> (ej a <-> b, but not 610 <-> 611)
> 
That ^ was my mistake.

Works as described, but still could not expand or collapse fields.
Comment 26 Bernardo Gonzalez Kriegel 2013-05-14 00:19:40 UTC
Also another side effect is that load time increases:
with this patch ~36 sec 
without ~9 sec
Comment 27 I'm just a bot 2013-09-18 09:58:14 UTC
Applying: Bug 7882 - move or reorder 6xx and 7xx fields
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/css/addbiblio.css
M	koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/css/addbiblio.css
Patch failed at 0001 Bug 7882 - move or reorder 6xx and 7xx fields
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 28 Kyle M Hall 2013-11-08 16:50:18 UTC Comment hidden (obsolete)
Comment 29 Kyle M Hall 2013-11-08 16:50:34 UTC Comment hidden (obsolete)
Comment 30 Kyle M Hall 2013-12-13 15:00:39 UTC Comment hidden (obsolete)
Comment 31 sandboxes@biblibre.com 2013-12-13 16:35:22 UTC
Patch tested with a sandbox, by Joel Sasse <jsasse@plumcreeklibrary.net>
Comment 32 sandboxes@biblibre.com 2013-12-13 16:36:15 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2013-12-24 11:24:36 UTC
QA comment:

After applying the patch, I am not able to add value in input text (as if they were readonly, but they are not...).

Moreover the commit message indicates that 6xx and 7xx fields will be sortable, but this patch impacts all fields .

Note that the page loading increases from 1.76s (onload: 5.19s) to 2.37s (onload: 7.15s) with this patch.

Marked as Failed QA.
Comment 34 Kyle M Hall 2014-01-07 12:36:19 UTC
> After applying the patch, I am not able to add value in input text (as if
> they were readonly, but they are not...).

Can you post some screenshots, or better yet, a video of this issue? I'm not able to reproduce it, but I'm not sure if I'm looking in the right place either.

> Moreover the commit message indicates that 6xx and 7xx fields will be
> sortable, but this patch impacts all fields .

Is this a real issue? That is, I propose we rename the bug to "move and reorder marc fields". I see this as only adding more flexibility to the marc editor.
Comment 35 Jonathan Druart 2014-01-07 13:36:58 UTC Comment hidden (obsolete)
Comment 36 Jonathan Druart 2014-01-07 13:39:16 UTC
(In reply to Kyle M Hall from comment #34)
> > After applying the patch, I am not able to add value in input text (as if
> > they were readonly, but they are not...).
> 
> Can you post some screenshots, or better yet, a video of this issue? I'm not
> able to reproduce it, but I'm not sure if I'm looking in the right place
> either.

I did nothing in particular, just open the editor and try to edit an input.
Why do you use disableSelection? My patch fixes my issue.

> > Moreover the commit message indicates that 6xx and 7xx fields will be
> > sortable, but this patch impacts all fields .
> 
> Is this a real issue? That is, I propose we rename the bug to "move and
> reorder marc fields". I see this as only adding more flexibility to the marc
> editor.

It is not an issue, of course. But it is not what it is written in the commit message.
Comment 37 Kyle M Hall 2014-01-13 12:50:05 UTC Comment hidden (obsolete)
Comment 38 Kyle M Hall 2014-01-13 12:50:26 UTC Comment hidden (obsolete)
Comment 39 Jonathan Druart 2014-01-13 13:15:13 UTC
Works as expected.

My only concern is about the page loading time. Tested now:

Before the patch:
1.12s (onload: 2.62s)
983ms (onload: 2.35s)
1.14s (onload: 2.5s)

After the patch:
1.6s (onload: 2.69s)
1.87s (onload: 3.04s)
1.59s (onload: 2.78s)

tested with firebug.

Maybe we should add a new pref here in order not to increase load time for low/old computers.

I let the RM takes the decision.

Marked as Passed QA.
Comment 40 Galen Charlton 2014-01-13 23:42:49 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #25)
> Works as described, but still could not expand or collapse fields.

This is still a problem -- ExpandField() cataloging.js, and possibly other routines, are expecting that subfields are contained in a <div>, not <li>, and would need to be updated.

Another thing that gets broken by the patch is the ability to client on the little triangle icons next to each subfield to move them up or down.  That should either be made to work, or it should be removed (on account that the subfields become sortable with this patch).

I'm failing the patch for those reasons, but they won't be difficult to deal with.

There's a more serious issue, however, which isn't caused by this patch but which would become more obvious with it -- if you open the record again in the editor, field order isn't maintained.  For example, if you construct your record so that subject headings are in the following order:

650 $aElizabethan theatre
600 $aShakespeare, William

then open it up again in the editor, the 600 will be displayed before the 650, even though the 650 field is stored first in the record.  This has the potential to be seriously confusing to users, and I feel that a patch that allows sorting of the fields in a user-specified order needs to be accompanied by work that makes the editor display existing fields in the order that they appear in the record.

The increase in the time (as measured by Jonathan) it takes to render the editor is unfortunate, but IMO does not quite rise to the level that calls for a new syspref.  Now if Bernardo's experience (36 seconds!) were the norm, that would be unacceptably slow.
Comment 41 Marjorie Barry-Vila 2015-02-12 19:17:54 UTC
Hi,
Someone is still working on it?

Regards,
Marjorie
Comment 42 Jacek Ablewicz 2015-03-30 08:51:59 UTC
(In reply to Marjorie Barry-Vila from comment #41)

> Someone is still working on it?

I wonder the same thing.. turns out that such feature would really come in handy, especially as C4::AuthoritiesMarc::merge() does currently have uncanny tendency to mess up field order in biblio records pretty badly, when authority records are being modified.
Comment 43 Marjorie Barry-Vila 2017-11-06 14:08:32 UTC
(In reply to Kyle M Hall from comment #38)
> Created attachment 24228 [details] [review] [review]
> Bug 7882: Inputs should be editable
> 
> disableSelection is deprecated and cause an issue on editing an input
> (input, select, textarea, etc.)
> see http://api.jqueryui.com/disableselection/
> 
> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Hi Kyle,

Do you think you can rebase the patch for the master version?

Regards,
Marjorie
Comment 44 David Bourgault 2018-02-28 14:50:14 UTC
>There's a more serious issue, however, which isn't caused by this patch but which would become more obvious with it -- if you open the record again in the editor, field order isn't maintained.  For example, if you construct your record so that subject headings are in the following order:
>
>650 $aElizabethan theatre
>600 $aShakespeare, William
>
>Then open it up again in the editor, the 600 will be displayed before the 650, even though the 650 field is stored first in the record.  This has the potential to be seriously confusing to users, and I feel that a patch that allows sorting of the fields in a user-specified order needs to be accompanied by work that makes the editor display existing fields in the order that they appear in the record.

This raises another question as to how to order the fields when loading a Marc record.

Let [1,2,3,4] be the framework structure
Let [1,4,2] be the saved record

Currently we load on the framework structure order, giving us
1, 2, 3 (empty), 4
This does not respect the saved record order. But if we load according do the saved record order, where should the empty tag (3) go?  Should we place it before the saved record's fields or after? Or do some weird half-sorted behavior?

before : [3 (empty), 1, 4, 2]
after  : [1, 4, 2, 3 (empty)]
weird  : [1, 3 (empty), 4, 2]

Let me know if my explanation isn't clear enough.
Comment 45 Katrin Fischer 2018-03-31 21:12:40 UTC
> 
> Let [1,2,3,4] be the framework structure
> Let [1,4,2] be the saved record
> 
> Currently we load on the framework structure order, giving us
> 1, 2, 3 (empty), 4
> This does not respect the saved record order. But if we load according do
> the saved record order, where should the empty tag (3) go?  Should we place
> it before the saved record's fields or after? Or do some weird half-sorted
> behavior?
> 
> before : [3 (empty), 1, 4, 2]
> after  : [1, 4, 2, 3 (empty)]
> weird  : [1, 3 (empty), 4, 2]

I'd put the empty ones at the bottom of the list, in alphabetic sequence.
Comment 46 Josef Moravec 2018-04-04 10:36:30 UTC
(In reply to Katrin Fischer from comment #45)
> > 
> > Let [1,2,3,4] be the framework structure
> > Let [1,4,2] be the saved record
> > 
> > Currently we load on the framework structure order, giving us
> > 1, 2, 3 (empty), 4
> > This does not respect the saved record order. But if we load according do
> > the saved record order, where should the empty tag (3) go?  Should we place
> > it before the saved record's fields or after? Or do some weird half-sorted
> > behavior?
> > 
> > before : [3 (empty), 1, 4, 2]
> > after  : [1, 4, 2, 3 (empty)]
> > weird  : [1, 3 (empty), 4, 2]
> 
> I'd put the empty ones at the bottom of the list, in alphabetic sequence.

+1
Comment 47 Maryse Simard 2019-10-23 19:36:58 UTC Comment hidden (obsolete)
Comment 48 Séverine Queune 2019-10-29 14:52:50 UTC Comment hidden (obsolete)
Comment 49 Jonathan Druart 2019-12-16 14:43:07 UTC
This is working great!

2 smalls things:
- required fields (the subfield's lib) are red
https://snipboard.io/CMuDYp.jpg
Is that expected?

- Do we really need the sort icon for each subfield?

And, not blocker for this bug report (regarding its age), but we could remove the "sortable" option for fields that cannot be duplicated for instance.
Comment 50 Jonathan Druart 2019-12-16 14:47:18 UTC
(In reply to Jonathan Druart from comment #49)
> This is working great!
> 
> 2 smalls things:
> - required fields (the subfield's lib) are red
> https://snipboard.io/CMuDYp.jpg
> Is that expected?

Sorry, that is coming from bug 24090.
Comment 51 Jonathan Druart 2019-12-16 14:58:31 UTC
Created attachment 96344 [details] [review]
Bug 7882: Add ability to move and reorder fields in MARC editor

Use jQueryUI sortable to make fields and subfields moveable in the
MARC editor for both records and authorities.

This patch convert items from div's to ul's and consequently alter css
and js to match. It also replace the up arrow with a more adapted icon
(from font awesome).

Primary authorship by Elliott Davis.

To Test:

1. Add or edit a record in cataloguing module.
2. You should be able to move the fields and subfields around.
    - You can click on any part of the element to drag it, the move
    icon to the left of the item is a good place to do it.
    => You can only change the order of fields of the same tag.
3. Make sure all of the javascript driven fonctionnality still work :
    - Duplicate fields/subfields
    - Remove fields/subfields
    - Using the tag editor for control fields or to link authorities
    - etc
4. Reorder some fields/subfields and save the record.
5. Edit the record again.
6. The order in the editor should match the changes which were saved.
    - Empty subfields should appear after the ones with content.
7. Repeat steps 1-6 with the authority editor.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 52 Jonathan Druart 2019-12-16 15:00:06 UTC
*** Bug 2124 has been marked as a duplicate of this bug. ***
Comment 53 Owen Leonard 2019-12-16 19:02:37 UTC
When I test this I am able to drag a tag but when I drop it it returns to its original position. Dragging subfields works to re-order them, but after I save the record the order is back to where it was before. Tested in Firefox and Chrome on Windows 10.
Comment 54 Owen Leonard 2019-12-16 19:13:36 UTC
Created attachment 96352 [details] [review]
Bug 7882: (follow-up) Update sorting interaction

This follow-up modifies the CSS for the sortable fields so that a
different icon is used. Elsewhere in Koha (stock rotation stage sorting)
we use the "bars" icon to indicate a sortable element. I think it would
be good to continue to use this. The cursor is now styled as a "Move"
cursor.

The configuration of the sortable function in the script has been
updated in two ways: 1. The movement of the element is constrained along
the y-axis, and 2. The movement of the element is contstrained to the
container within which it can be moved (for the tag, the tab, and for
the subfield, the tag).

To test, apply the patch and edit a MARC record in the basic cataloging
editor.

Confirm that the new icon is used, and that the "Move" cursor shows when
you hover over the icon.

Confirm that a tag can be dragged up and down within the constraints of
the visible section. A subfield should be only movable within its parent
tag.
Comment 55 Owen Leonard 2019-12-16 19:14:48 UTC
I've added a minor follow-up even though this isn't working correctly for me (comment 53).
Comment 56 Jonathan Druart 2019-12-17 09:20:17 UTC
(In reply to Owen Leonard from comment #53)
> When I test this I am able to drag a tag but when I drop it it returns to
> its original position. Dragging subfields works to re-order them, but after
> I save the record the order is back to where it was before. Tested in
> Firefox and Chrome on Windows 10.

Hi Owen,
The important bit is "    => You can only change the order of fields of the same tag."
That is always why I suggested to remove the sortable thing for those fields. For instance unique fields cannot be moved.

> Dragging subfields works to re-order them, but after I save the record the order is back to where it was befor

I do not see this issue. I can sort subfields, save, edit again => the subfields order is kept.
Comment 57 Owen Leonard 2019-12-17 12:47:23 UTC
(In reply to Owen Leonard from comment #55)
> even though this isn't working correctly for me

You're right Jonathan, tester error.
Comment 58 Owen Leonard 2019-12-17 15:23:53 UTC
Created attachment 96384 [details] [review]
Bug 7882: (follow-up) Only show sortable icon for sortable tags

Only tags which can be repeated can be sorted, so the "bars" icon should
not display by these tags.
Comment 59 Jonathan Druart 2019-12-20 08:17:01 UTC
Maryse, Séverine, could you signoff Owen's patch?
Comment 60 Maryse Simard 2019-12-20 22:27:36 UTC
Created attachment 96541 [details] [review]
Bug 7882: (follow-up) Update sorting interaction

This follow-up modifies the CSS for the sortable fields so that a
different icon is used. Elsewhere in Koha (stock rotation stage sorting)
we use the "bars" icon to indicate a sortable element. I think it would
be good to continue to use this. The cursor is now styled as a "Move"
cursor.

The configuration of the sortable function in the script has been
updated in two ways: 1. The movement of the element is constrained along
the y-axis, and 2. The movement of the element is contstrained to the
container within which it can be moved (for the tag, the tab, and for
the subfield, the tag).

To test, apply the patch and edit a MARC record in the basic cataloging
editor.

Confirm that the new icon is used, and that the "Move" cursor shows when
you hover over the icon.

Confirm that a tag can be dragged up and down within the constraints of
the visible section. A subfield should be only movable within its parent
tag.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Comment 61 Maryse Simard 2019-12-20 22:27:47 UTC
Created attachment 96542 [details] [review]
Bug 7882: (follow-up) Only show sortable icon for sortable tags

Only tags which can be repeated can be sorted, so the "bars" icon should
not display by these tags.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Comment 62 Jonathan Druart 2019-12-26 11:33:55 UTC
Created attachment 96633 [details] [review]
Bug 7882: Add ability to move and reorder fields in MARC editor

Use jQueryUI sortable to make fields and subfields moveable in the
MARC editor for both records and authorities.

This patch convert items from div's to ul's and consequently alter css
and js to match. It also replace the up arrow with a more adapted icon
(from font awesome).

Primary authorship by Elliott Davis.

To Test:

1. Add or edit a record in cataloguing module.
2. You should be able to move the fields and subfields around.
    - You can click on any part of the element to drag it, the move
    icon to the left of the item is a good place to do it.
    => You can only change the order of fields of the same tag.
3. Make sure all of the javascript driven fonctionnality still work :
    - Duplicate fields/subfields
    - Remove fields/subfields
    - Using the tag editor for control fields or to link authorities
    - etc
4. Reorder some fields/subfields and save the record.
5. Edit the record again.
6. The order in the editor should match the changes which were saved.
    - Empty subfields should appear after the ones with content.
7. Repeat steps 1-6 with the authority editor.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 63 Jonathan Druart 2019-12-26 11:34:00 UTC
Created attachment 96634 [details] [review]
Bug 7882: (follow-up) Update sorting interaction

This follow-up modifies the CSS for the sortable fields so that a
different icon is used. Elsewhere in Koha (stock rotation stage sorting)
we use the "bars" icon to indicate a sortable element. I think it would
be good to continue to use this. The cursor is now styled as a "Move"
cursor.

The configuration of the sortable function in the script has been
updated in two ways: 1. The movement of the element is constrained along
the y-axis, and 2. The movement of the element is contstrained to the
container within which it can be moved (for the tag, the tab, and for
the subfield, the tag).

To test, apply the patch and edit a MARC record in the basic cataloging
editor.

Confirm that the new icon is used, and that the "Move" cursor shows when
you hover over the icon.

Confirm that a tag can be dragged up and down within the constraints of
the visible section. A subfield should be only movable within its parent
tag.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 64 Jonathan Druart 2019-12-26 11:34:05 UTC
Created attachment 96635 [details] [review]
Bug 7882: (follow-up) Only show sortable icon for sortable tags

Only tags which can be repeated can be sorted, so the "bars" icon should
not display by these tags.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 65 Martin Renvoize 2020-01-03 16:30:49 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 66 Joy Nelson 2020-01-14 00:53:11 UTC
not backported to 19.11.x
Comment 67 Fridolin Somers 2020-03-23 11:50:56 UTC
*** Bug 11073 has been marked as a duplicate of this bug. ***
Comment 68 Joy Nelson 2020-04-03 19:45:40 UTC
Was asked to reconsider backporting.  This is now backported to 19.11.x branch for 19.11.05
Comment 69 Lucas Gass 2020-04-13 16:58:07 UTC
choosing to backport this very cool enhancement. 

backported to 19.05.x for 19.05.10
Comment 70 Hayley Mapley 2020-04-15 01:22:48 UTC
Enhancement will not be backported to 18.11.x series
Comment 71 Father Vlasie 2020-11-06 04:16:15 UTC
This feature does not work in Safari (tested with version 14). Perhaps we could have the older arrow system for browsers that do not support this otherwise some users are without a method to reorder subfields. Thank you!