Web Client: need way to delete volume added in vol editor

Bug #1739087 reported by Elaine Hardy
18
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Undecided
Unassigned
3.0
Won't Fix
Undecided
Unassigned
3.1
Fix Released
Undecided
Unassigned

Bug Description

In Holdings view, if I am cataloging for multiple branches, if I accidentally add a volume for a branch, there is no easy way to delete the volume. In the XUL client, you can set the number of vols to 0 and proceed with the remaining vols. In the web client, setting the vol to 0 does not allow you to proceed with adding the remaining vols and copies. You cannot save even with the vol set to 0. The only solution I have found is to redisplay the vol/copy editor while the vol is 0. That, however, resets any values you have added to the copies/volumes and you have to start all over.

I suggest an x beside each volume that allows you to remove it from the volume list, or to honor the 0 count and ignore the volume as in the XUL client.

Revision history for this message
Kathy Lussier (klussier) wrote :

I like the idea of using an x.

tags: added: webstaffclient
Revision history for this message
Beth Willis (willis-a) wrote :

The only other way I can see to delete an unwanted volume is to increase the volume count to the total number of volumes added, then decrease it to the number of desired volumes. So, for example, if I have added 2 volumes/copies and inadvertently add a third copy for the wrong branch, I could increase the volume count to 3 before decreasing it back to 2. The third copy is then deleted.

However, I agree that it would better to add an X after each volume to make deletion more straightforward.

Revision history for this message
Andrea Neiman (aneiman) wrote :

Confirmed on 3.0.3 test system. Agreed with the idea of using an x on each line.

Changed in evergreen:
status: New → Confirmed
tags: added: cataloging
Cesar V (cesardv)
Changed in evergreen:
assignee: nobody → Cesar V (cesardv)
Revision history for this message
Cesar V (cesardv) wrote :

Here's a possible fix implementing an "X" next to any delete-able volumes in the VolCopyEditor.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/cesardv/lp1739087_volcopy_x_cn

Revision history for this message
Cesar V (cesardv) wrote :

Attached a preview of how it looks...

tags: added: pullrequest
Andrea Neiman (aneiman)
Changed in evergreen:
milestone: none → 3.2-beta
Revision history for this message
Jason Etheridge (phasefx) wrote :

Cesar, I tried the following in concerto with the patch:

Cataloging -> Retrieve Bib Record by ID -> 1
Add Copies
To the left of Add Volume, changed the entry to BR2 and then clicked Add Volume
To the left of Add Volume, changed the entry to BR3 and then clicked Add Volume
Clicked the X for the BR2 entry

This leaves a row for BR2, but only with the Owning Library and Volumes columns having widgets. The Volumes widget is set to 0. Incrementing the value to 1 will not re-vivicate the row, but continuing on to 2 will create two rows for BR2.

Another cosmetic bug is that you can manually enter 0 in the Volumes field for an "Add volume" row and then click X on the row, and the 0 will change to -1.

Revision history for this message
Cesar V (cesardv) wrote :

Thanks for testing this Jason!
I've pushed another commit to the top of the branch I submitted.
Could you give it another shot?
It should address the 2 issues you reported.

Revision history for this message
Jason Etheridge (phasefx) wrote :

Thanks Cesar! I haven't been able to get it to glitch at all now. If you enter 0 or a negative number manually in the widget, they get ignored, which I suppose is okay, though some catalogers may like the whole 0 being treated the same as if you clicked the Delete icon.

The only concern I have now is accessibility and I18N. We have a very stylized and non-localized "x" (using inline CSS no less, instead of a CSS class), and nothing telling screen readers/etc. what it is for (though I can't imagine the interface as a whole is very friendly to screen readers). I'm not sure of the best solution here or how critical it is.

Revision history for this message
Jason Etheridge (phasefx) wrote :

It looks like the attribute aria-label may be the way to go on the button element.

Revision history for this message
Jason Etheridge (phasefx) wrote :

To be fair, the >X< pattern is used elsewhere in the code, so I'd be okay letting this go and having a separate bug tackle all such uses at once.

Revision history for this message
Cesar V (cesardv) wrote :

Thanks for re-testing this Jason!
Right, so since that numeric input allows any sort of input, i.e users can enter zero or any negative values, yet those values don't have any effect and are "validated" by the constraints in place... I think entering 0 there manually should become a moot point, esp. since the new "X" button is tab-able and allows for better control of which volumes to delete. But then again I'm not a cataloger so that's just my two cents :)

Anyhow, I've added an aria-label="Delete" attribute to the X button that will permit accessibility devices to distinguish the delete button's function. Regarding the inline CSS... well I figured since the egVolRow directive has an inline template, I might was well just inline that small bit of CSS as well so that if/when these get rewritten/refactored into a proper/separate TT files, then the proper CSS rules are put in on it's own separate file too. I confess I do get a bit lost when trying to find the proper CSS file for a particular rule, but I am not opposed to creating a CSS rule for it however, since that's usually the way to go most times.

Revision history for this message
Remington Steed (rjs7) wrote :

Hi Cesar,

I'd actually like to see how the "X" looks without the CSS rules. My instinct would be to have the "X" as a full-size button that lines up with the rest of the form elements, similar to the lines of the OPAC Advanced Search form that can be added and removed. Of course, that might require styling also. I don't have any wisdom about where styles should go in this situation.

Revision history for this message
Elaine Hardy (ehardy) wrote :

Chris added this to the PINES test server and I just test

When I add a volume/copy using the add copies button, or selecting one library from the holdings view list and then add another vols in the Vol/copy editor, I can delete the vols for all but the first library added.

Workflow: Click on add copies button. Vol/copy editor opens with LIB-A vol to add. use add vol button to add vol to LIB-B. LIB-B has X for deletion (and can delete) but LIB-A does not.

If I select libraries from holdings view (LIB-A, LIB-B, LIB-C) and then from actions menu choose add volumes and copies, no X appears in the vol/copy editor and I cannot delete a volume.

Revision history for this message
Cesar V (cesardv) wrote :

hey Remington! Actually, the Bootstrap "X" button is literally just a times/multiplication symbol "&times;" inside a <button> element. Apart from the "margin:-5px -15px; float:left;" inline CSS I added to move the X to the left (otherwise it'd stack on top of the classification field), the only other CSS comes from the Bootstrap library's CSS classes "close" and "button.close" that takes care of the styling, giving it the look and feel of a "close" button. I suppose we could override that to our hearts content, but I just went with the default button look Bootstrap gave me and just used a negative margin trick to position it on the left before the input field.

Hi Elaine, so as far as I understand it, one can only delete the ephemeral volumes in the VolCopy editor, i.e those created after the page loads.

Revision history for this message
Elaine Hardy (ehardy) wrote :

We need to be able to delete any of the volumes in the editor rather than having to close it and start again

Revision history for this message
Mike Rylander (mrylander) wrote :

Hi Elaine,

The vol/copy edit interface isn't currently designed to provide a delete action for anything. As far as I can tell, this was not possible in the XUL interface either; you can only delete existing volumes only from the explicit Delete Volumes action in the holdings list.

In the interest of getting a significant improvement into 3.2 (and, hopefully, being able to backport it), can we separate the new function of deleting existing volumes to a new wishlist bug, and get this fix in for deleting "ephemeral" volumes added during an edit session?

Thanks!

Revision history for this message
Elaine Hardy (ehardy) wrote :

You can delete volumes in the editor in the XUL client. Setting the counter to 0 effectively removes or deletes the volume from the editor

Revision history for this message
Elaine Hardy (ehardy) wrote :

I think we would be OK with the first workflow where you can't delete the first volume added:

Workflow: Click on add copies button. Vol/copy editor opens with LIB-A vol to add. use add vol button to add vol to LIB-B. LIB-B has X for deletion (and can delete) but LIB-A does not.

However, PINES catalogers would rarely be using this method and would most likely be selecting multiple libraries to batch add vols and items. Is there anyway to at least set the number of vols to 0 as in the XUL client, so that no vol is added? Otherwise, cats would have to either close the editor and start all over or add a fake vol and copy and delete it immediately.

While this isn't an error that occurs often, it does enough to need an easy solution. Even if maintaining the ability to set the incorrectly added vol to 0 and be able to save & exit the editor

Revision history for this message
Jason Etheridge (phasefx) wrote :

Elaine, I got you. I've been conflating creating and editing and "real" deletion with ephemeral UI deletion over here in my head, so it got jumbled.

Revision history for this message
Elaine Hardy (ehardy) wrote :

Thanks!

Revision history for this message
Mike Rylander (mrylander) wrote :

Elaine,

Mea culpa, I was missing the use case the Jason restated. I think my confusion was because of the word "delete" -- I misunderstood that you were looking to remove from the UI rather than delete the backing object.

Revision history for this message
Elaine Hardy (ehardy) wrote :

Mike,

Mea culpa as well -- I think of this as deleting when it is better described as removing it from a UI, before it is actually added to the database.

To restate, since I think we are all on the same page now:

We need to be able to remove a volume incorrectly added to the list in the vol/item editor, before it is saved.

Currently, if we don't associate a barcode with all volumes in the list, we cannot save & exit. If we add a vol in error, we have to either close the editor and start over or add the incorrect vol/copy and immediately delete it from the database.

Either having a way to set the counter of volumes in the editor to zero so that the volume is ignored (as in the XUL client) or a way to remove the volume using an "x" beside the volume would solve the issue.

Thanks!

Revision history for this message
Cesar V (cesardv) wrote :

I've added another commit on top of the already mentioned branch that would allow for Elaine's use case.
Sorry for the confusion.

Revision history for this message
Mike Rylander (mrylander) wrote :

One more note: I believe a workaround for the time being, especially when adding copies (either with or without new volumes) would be to use the Store Selected action for only those copies one wants to save, ignoring the mistaken volumes (and copies), and then save the copies from the Completed Copies tab.

Revision history for this message
Elaine Hardy (ehardy) wrote :

I added five volumes, edited all. Stored the four I wanted to keep. Clicked on completed copies and then the drop down menu Save -- completed. Nothing happens.

I believe this is bug https://bugs.launchpad.net/evergreen/+bug/1787483

Revision history for this message
Jason Etheridge (phasefx) wrote :

Cesar, everything is working for me, though I did trip over a bug that also exists in master. I pushed a sign-off branch to collab/phasefx/lp1739087_volcopy_x_cn

The new bug is bug 1789504. Elaine, I wonder if this what you tripped over instead of bug 1787483?

Kathy Lussier (klussier)
Changed in evergreen:
assignee: Cesar V (cesardv) → nobody
Andrea Neiman (aneiman)
tags: added: signedoff
Revision history for this message
Jason Etheridge (phasefx) wrote :

I added an extra commit on top of my sign off branch to try to address bug 1731370

Revision history for this message
Jason Etheridge (phasefx) wrote :

That extra commit wasn't as straightforward as I had hoped, so here's the pristine sign-off branch for Cesar's work again: collab/phasefx/lp1739087_volcopy_x_cn_orig

I think this can go into master as is.

Changed in evergreen:
milestone: 3.2-beta → 3.2-rc
Changed in evergreen:
assignee: nobody → Jason Etheridge (phasefx)
Changed in evergreen:
assignee: Jason Etheridge (phasefx) → nobody
Changed in evergreen:
milestone: 3.2-rc → 3.2.1
Revision history for this message
Elaine Hardy (ehardy) wrote :

With this applied, I'm not seeing any way to remove a vol from the vol editor,

Workflow 1:
1) choose libs in holdings view by checking boxes next to libs, choose add vols and copies from action menu.
2) vol/copy editor opens.
3) no check box for removal and setting vol to zero while trying to add STATELIB-A office copy doesn't work
4) Save & exit greyed out

Workflow2:
1) Click add copies button
2) Editor opens with STATELIB-A vol to be added
3) add vol for STATELIB-L
4) no check box for removal and setting vol to zero while trying to add STATELIB-A office copy doesn't work
5)Save & exit greyed out

Revision history for this message
Galen Charlton (gmc) wrote :

Elaine: I tested this against master as of today, including the patches I pushed for bug 1746536, and I'm now seeing the X's. Could you test again?

Changed in evergreen:
milestone: 3.2.1 → 3.2-rc
status: Confirmed → Fix Released
Revision history for this message
Elaine Hardy (ehardy) wrote :

Let me ask Chris to add this to our test server. He is training today so it may not until tomorrow.

Thanks!

Revision history for this message
Galen Charlton (gmc) wrote :

I've also now backported this to rel_3_1 for inclusion in 3.1.7, but will also be asking for additional eyes to crosscheck before the release.

Revision history for this message
Christine Burns (christine-burns) wrote :

tested on our 3.1.7 test server and I am seeing the X's.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.