Angular: Local Admin Shelving Location Groups Port

Bug #1852321 reported by Mike Risher on 2019-11-12
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Wishlist
Unassigned

Bug Description

Evergreen: master

We’ll be porting the Shelving Locations Group UI from DOJO to Angular. This will involve usage of the Org Selector, and will have three columns with a list of items preceded by a checkbox.

Column 1: Location Groups (Option to create a new one using FM Editor)

Column 2: Group Entries (Button that says Remove ->)

Column 3: Shelving Locations (Button that says ← Add)

Hitting add will move items from Shelving Locations to Group Entries. Remove does the opposite.

Relevant files from the DOJO UI can be found here:

TT2: Open-ILS/src/templates/conify/global/asset/copy_location_group.tt2

Js: Open-ILS/web/js/ui/default/conify/global/asset/copy_location_group.js

Mike Risher (mrisher) on 2019-11-12
tags: added: angular
removed: angul
Changed in evergreen:
assignee: nobody → Mike Risher (mrisher)
Mike Risher (mrisher) wrote :
Changed in evergreen:
assignee: Mike Risher (mrisher) → nobody
tags: added: pullrequest
Bill Erickson (berick) on 2019-12-03
Changed in evergreen:
milestone: none → 3.5-alpha
status: New → Confirmed
importance: Undecided → Wishlist
Mike Risher (mrisher) wrote :

In response to our demo today I did a little research on the correct cursor to show when an element is draggable. I saw a couple different views, but the primarily accepted one seems to be use 'move' (4 arrows) to re-order items, or 'grab' (a hand) to do an operation of some sort on the item. I went with the 'move' cursor. Now when hovering the mouse over any location group the cursor will change. Happy to use a different cursor if the community likes.

My branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1852321-shelving-locations-group

Jennifer Pringle (jpringle-u) wrote :

The Add/Remove functions work well for me but I ran into some issues when creating new shelving location groups.

The New Location Groups form will allow you to save without filling in any of the fields and returns the message Update Succeeded in green in the bottom right corner. You can also save if you just leave one of the fields, like name or org unit, blank. In these cases no new group displays in the client; I don't have the ability to check if anything is being created in the database in these cases.

I'd suggest renaming the Owning Org Unit field to either Owning Library or Org Unit for consistency. Skimming through the local admin menu options Owning Library seems to be a bit more commonly used than Org Unit though we have lots of variation currently.

I'm unclear about whether Position should be an editable field. In the old version when creating a new group this field is populated based on what the next position in the list is and is grayed out. When editing groups this field doesn't display at all and the order of groups is rearranged only through drag and drop. In the Angular version you can enter any number, including one already used by another group, and save. You can rearrange the order of the list by drag and dropping or by editing and updating the value for Position. The drag and drop function doesn't work in my testing on a touchscreen (in either version) so I think there is some value in being able to manually edit the position.

Ruth Frasur (rfrasur) wrote :

Entering a nonalphanumeric character in name field seems to hang up the saving/creating process.

Ruth Frasur (rfrasur) wrote :

This is not necessarily bad behavior, but there should probably be a screen tip or documentation stating that only alphanumeric characters should be used.

Mike Risher (mrisher) wrote :

Re: non alphanumeric fields

I'm unable to confirm. I created a new location group with the name "/", one with the name "☆☆☆☆☆" and another one with the name "ךךךך". (Hebrew characters)

Which non alphanumeric character is it rejecting on your end?

Mike Risher (mrisher) wrote :

Thanks for the feedback, Jennifer Pringle. I addressed most of the issues you raised. Here are the ways the work has been improved:

* The location group has a new field showing its position.
* To make it clear what that new field is, there are now column labels at the top of the list of location groups
* When creating a location group "name" is now required. It's not possible to save without a name.
* When creating a new location group the org is set the same as the current org selected at the top of the screen. It can't be edited.
* When creating a new location group the position defaults the highest position in use, plus one. This will result in it always going to the bottom of the list unless you enter a new value. (I looked at copying the behavior of the old UI, but in my testing it kept defaulting to values that didn't make sense.)

I have not yet adjusted the name "Owning Org Unit". I wanted to get these changes in during feedback fest week, and the name change will take more time.

Work is on this branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1852321-shelving-locations-group-followup

Mike Risher (mrisher) wrote :

One more detail I forgot:

* You cannot save the record unless you have a value for Position.

Mike Risher (mrisher) wrote :

The label has been updated to "Owning Library". I think I've addressed everything and this is ready for another look.

Same branch:
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1852321-shelving-locations-group-followup

Jane Sandberg (sandbej) wrote :

Overall, I like this interface a lot, Mike!

There are two issues that I noticed:

1) Each entry in the "Group Entries" and "Shelving Locations" column consists of a checkbox and the name of the location. However, the checkbox and label are not associated with each other. This is a critical accessibility issue; could you please associate them with each other? More info here: https://dequeuniversity.com/rules/axe/3.2/label

2) If I don't have any groups defined, and I click on "New Location Group", nothing happens. I get this error in the console: ERROR TypeError: "this.locationGroups[0] is undefined"

I also want to support having Position as an editable field, as Jennifer noted in comment #4. Drag and drop interfaces have some serious accessibility considerations for blind users, users who navigate via keyboard, and users with motor disabilities. I'm glad that we have the editable position field as another way to set the order of these groups, in addition to the drag-and-drop interface.

Mike Risher (mrisher) wrote :

Thanks so much for the additional feedback, Jane! I've addressed both issues. The branch is the same:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1852321-shelving-locations-group-followup

Changed in evergreen:
milestone: 3.5-beta → 3.5.0
Changed in evergreen:
milestone: 3.5.0 → 3.5.1
Jane Sandberg (sandbej) on 2020-07-21
Changed in evergreen:
milestone: 3.5.1 → 3.6-beta
Mike Rylander (mrylander) wrote :

This is working like a champ for me, FWIW. Will look to sign off if nobody beats me to it.

Changed in evergreen:
milestone: 3.6-beta → 3.next
Chris Sharp (chrissharp123) wrote :

Removed pullrequest - code needs to be updated for Angular 10, per Bill Erickson in IRC (http://irc.evergreen-ils.org/evergreen/2020-09-25#i_460263 and following).

tags: removed: pullrequest
Jennifer Weston (jweston) wrote :

Thanks, Mike, Jane, and Jennifer P. -
Tested on https://tiffany-master.gapines.org/eg/staff/

Functionality is working as expected, including all revisions based upon feedback.

I have tested this code and consent to signing off on it with my name and my email address, Jennifer Weston, <email address hidden>

tags: added: signedoff
Michele Morgan (mmorgan) on 2020-11-03
Changed in evergreen:
milestone: 3.next → 3.7-beta
Changed in evergreen:
assignee: nobody → Chris Sharp (chrissharp123)
Galen Charlton (gmc) wrote :

Pushed to master for inclusion in 3.7. Thanks, Mike and Jennifer.

(Sorry, Chris, I didn't notice until I pushed it that you had claimed this one back in February)

Changed in evergreen:
assignee: Chris Sharp (chrissharp123) → nobody
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers