webclient: Need ability to type to selection in some menus

Bug #1511742 reported by Kathy Lussier
64
This bug affects 13 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
High
Unassigned

Bug Description

In the existing client, we have the ability to start typing the name of an org unit in the OU menus. Although we can type to a selection in most webclient menus, we are unable to do so in the org unit selector. I'm guessing the problem is due to the hierarchical nature of this selector. We'll need some way of allowing users to type to a selection in menus of a hierarchical nature in the web client.

Revision history for this message
Kyle Huckins (khuckins) wrote :

You're spot on with the issue being related to the hierarchal nature of the OU selector. To allow for the hierarchical view, the dropdown is actually a hidden dropdown list, activated on pressing the OU field, which is actually a button. Before commiting to assigning myself to this issue, I've run through a few ideas:

-Using a regular select field - this loses the hierarchical view.
-Changing the button to a text input with a typeahead dropdown, retaining the original unordered list - This hasn't seemed to work so far, unfortunately.
-Utilizing a datalist - this, once again, loses the hierarchical view, and really doesn't get us what we want.

I've noticed that a few areas, particularly in the administration panel(for example, creating a new billing type), have an OU dropdown that works as we'd want it to. The problem being that it seems to be a direct port from the existing staff client, and utilises dojo.

Kyle Huckins (khuckins)
Changed in evergreen:
assignee: nobody → Kyle Huckins (khuckins)
Revision history for this message
Bill Erickson (berick) wrote :

Kyle, to clarify a bit, the interfaces that use Dojo are not ported / new interfaces. They are the same as those used by the XUL client, just embedded in the browser client as a temporary stop-gap.

General question, if we had to make a decision, which is more important, type-to-find or formatting/indentation? Would a traditional flat selector be better than what we have now?

Revision history for this message
Kyle Huckins (khuckins) wrote :

Thanks for the clarification.

On another note, I've made some progress in getting this to work with both hierarchal indentation and the autocompletion, which I'll push up after a bit more testing and making sure I haven't missed any interfaces which might need adaptation. There's a caveat with this, though - In order to get it to work, the OU button has to be an input field, which required some small changes to the workstation administration page - nothing major, but the register a new workstation section had to be broken up a bit. If needed, I can post a screenshot.

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

Kyle, since you're (presumably) working on a very widely used widget, I'd appreciate a push of the code as-is so we can help spot any lurking trouble early, if there is any. TIA

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

Thanks, Kyle. I have one question, and it may come down to style: does 'placeholder="{{label}}"' not work, and require the new getLabel() function, or was that a preference on your part? I ask because, for the most part, we use scope variables directly rather than wrapping them in accessor functions, except for "internal use" variables like underscore-prefixed ones.

Thanks!

Revision history for this message
Kyle Huckins (khuckins) wrote :

D'oh. I'd tried several things while working on this, and at some point I fell into a coding trance - don't think I even considered the obvious and simple choice of going with placeholder="{{label}}". I'll make the change and test it just in case. Thanks for pointing that out!

Revision history for this message
Kyle Huckins (khuckins) wrote :

placeholder="{{label}}" worked, and I've amended the commit to get rid of the unnecessary function.

Revision history for this message
Kyle Huckins (khuckins) wrote :

I've amended my previous commit with a more complete version. There's one last thing I can think to add here, and that's a warning for when the value of the input field is incomplete/incorrect. Currently the behavior is to default to the last selected OU, but it might be good to make sure the user knows that.

Examples of this situation would be when the input value is "CONSS" instead of "CONS" or "B" instead of "BR1." In that first example, once CONS is typed in, CONS will be selected automatically, so it will default to CONS. In the second example, it will default to whatever the last valid OU that was input or selected was.

My current thought is to apply a warning to the field if the current input if unfocused and the value doesn't match the shortnames of any valid OUs.

Revision history for this message
Bill Erickson (berick) wrote :

Did a quick test and found I was able to both see the indented list when clicking within the input field and type-to-find values as expected.

+1 to styling the input when an invalid value is set.

Revision history for this message
Kyle Huckins (khuckins) wrote :

I've added the invalid value styling, and caught and fixed an issue where the scrollbar would no longer appear as expected, and amended my previous commit with the changes.

Revision history for this message
Kyle Huckins (khuckins) wrote :

I've made some more progress on this, and found several interfaces that needed slight modification to play nice with the changes to the input field. Most of these are minor, just adding proper scaffolding in places that did not previously have it. I did notice some difficulties in certain interfaces in the local administration settings, where the link function was insisting scope.egAuth.user() was null. I also noticed that I was able to access that section without being logged, which may be related(maybe it's not caring about egAuth at all).

I've also rebased and made sure everything was working fine with the latest updates to master. It's on the same branch, but to save any scrolling, here's a link to the branch: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/khuckins/lp1511742-ou-menu-type-select

Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Kyle. Can you please provide examples of what interfaces you are able to access without logging in? That should not be happening, so we'll probably need a new bug for those.

Revision history for this message
Kyle Huckins (khuckins) wrote :

Testing on this branch, and then on master to confirm, I was able to reach these interfaces without being logged in:

-Address Alerts(Not always)
-Age Circulations to Lost
-Auto-Print Settings
-Patrons with Negative Balances
-Statistical Popularity Badges

Revision history for this message
Kyle Huckins (khuckins) wrote :

Trying some more, I may just be imagining the Address Alerts interface allowing me to access it while not logged in.

Revision history for this message
Bill Erickson (berick) wrote :

Opened https://bugs.launchpad.net/evergreen/+bug/1653998 for the UI's failing to redirect the user to the login page.

Revision history for this message
Kyle Huckins (khuckins) wrote :

Just realized I hadn't quite updated the branch correctly, somehow managed to throw in a slightly newer, but still outdated version. This branch now has the latest updates to this code, and should be good to go

Changed in evergreen:
assignee: Kyle Huckins (khuckins) → nobody
tags: added: pullrequest
Changed in evergreen:
assignee: nobody → Jennifer Pringle (jpringle-u)
Revision history for this message
Kathy Lussier (klussier) wrote :

Hi Kyle,

I know that Jennifer is all set to test this next week, but I wanted to mention something I noticed when I recently loaded the code on my server, but ran out of time to test it. In the previous selector, I noticed there was a drop down arrow that informed the user that the selector was a drop down menu. At the moment, it looks similar to a free-text entry box.

I don't think it's a big deal, but it's a nice UI feature to have for the web client drop down menus. Is that something that can be integrated into the new menus?

Thanks!
Kathy

Revision history for this message
Kyle Huckins (khuckins) wrote :

Hey Kathy,

The menus actually are free-text entry boxes, making use of typeahead to simulate a dropdown. This was done in order to preserve the spacing and the look & feel of the already existing dropdowns. It may take some reworking to bring in the dropdown arrow. I've made some attempts to get the arrow to display, but none of them have worked out all that well so far. Very much agreed that it's a nice thing to have.

One thought that I have(which would still be more than a minor change) is to have a separate button that visually seems attached to the text field(similar to the old workstation registration field. It would display as a dropdown arrow, and trigger the full dropdown when pressed. I'm not sure how this idea would work with certain accessibility tools that may be used, like screen readers, however.

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

I'd go further and say that the dropdown is a critical need. If you don't know the specific branch name you're looking for, but you know, say, the system level name, you'll have a hard time finding what you need, no?

I think the basic idea you outline, of a down-arrow button that opens the dropdown, would work well. You might want to take a look at the dropdown for copy templates in the volume/copy editor, and for copy parts in that same interface. They work in a way similar to what you describe. They don't indent today, but they do provide both type-ahead and full-dropdown capabilities.

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

Sorry for the confusion on this Mike! I don't have the code loaded on a server currently, but my recollection is that the list of Org Units do indeed appear once you click in the free-text entry box. The question I raised was more of a visual cue need than a functional need. But, like I said, I'm not look at this code right now, so Kyle, feel free to correct me if I'm misremembering.

Revision history for this message
Kyle Huckins (khuckins) wrote :

Yes, clicking on the text input will load the list of orgs, so the dropdown itself does exist. The problem being the arrow doesn't.

Looking at the template dropdown in the volume/copy editor tt2, it's doing a call to the eg-basic-combo-box directive, followed by a button element. If we want the arrow, it should just be a matter of adding a button into the eg-org-selector directive(with some tweaks to make sure plays nice with scaffolding).

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

Ah! I should look before commenting. I misunderstood, sorry for the noise!

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

Hi Kyle,
This branch has a merge conflict. Could you resolve it and push a rebased branch?

Thanks!

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

Just a quick note on the branch as it stands ... this will require adjustment to know about offline mode, or it will break that interface. Specifically, we can't count on egAuth.user() existing because offline mode forces a logout, so $scope.isValidOU will probably need to answer "yes" to all in that case.

Consider this comment a placeholder that whichever branch goes in last will need to adjust the org tree widget.

Revision history for this message
Kyle Huckins (khuckins) wrote :

I'm taking a look at this now, and have done most of the rebasing on a local branch, now I'm taking a look at what changes will be needed to accommodate offline mode, which seems simple enough. $scope.isValidOU will definitely need changes, there's also another small bit of code in the egStartup.go().then() function setting $scope.selected to egCore.auth.user()'s workstation org unit if there's no current selection or default - I believe this will need to be modified as well in scenarios where there's no user(so, offline mode).

Are there any scenarios where a user wouldn't exist outside of offline mode?

Revision history for this message
Kyle Huckins (khuckins) wrote :

Per request, the branch has been rebased and it should now work alongside offline mode by checking for the existence of a user, and in absence of a user, defaulting to the highest tier org unit when there is no set default, and assuming true in isValidOU().

Branch is the same as it previously was, located here: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/khuckins/lp1511742-ou-menu-type-select

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

Hi Kyle,

I can't actually pick the top commit since it's a merge commit. Perhaps my git-foo is simply lacking, but this doesn't look like a normal rebase on top of master... Help?

TIA

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

Kyle,

Jason Stephenson helped my find the commits ... my git-fu is lacking, it turns out. There seem to be two commits using git-cherry, but neither of those contains the offline updates. Can you rebase rather than merge so that your code is sitting at the top of a branch directly on top of master, please?

Thanks!

Revision history for this message
Kyle Huckins (khuckins) wrote :

Ooof, my bad, my git-fu is lacking as well. The latest push should be what you want

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

Thanks, Kyle! That looks much better. The top 2 commits pick cleanly onto current master.

Can you describe the problems that the top commit fixes, and what the solution is doing? For instance, you have a change to how the User Profile dropdown is built in the age_to_lost template, which seems unrelated to the org selector. I assume it's simply a matter of needing to put more structure around any use of the org selector, but I'm not sure what the problem actually is.

My concern is that there will be many places in other topic branches (ironically, offline among them) that will need to be adjusted, and I wonder if we might be able to fix the issues inside the eg-org-selector template itself, instead of at all the call sites. Perhaps the org selector needs a default (and overridable-via-attribute) min or max width?

Revision history for this message
Kyle Huckins (khuckins) wrote :

Hey Mike!

So the top commit handles scaffolding changes that needed to be including in order for the new input to display correctly. Without it, there would be oddities in how the org selector would display - width being too small, width taking up the entire screen, positioning, etc. While the width may be able to be handled, bootstrap standards generally prefer things to be handled via columns, and the positioning changes that come along with the scaffolding additions would be tough to do in the template. At that point it seemed simpler to just work with the necessary scaffolding changes on each interface, making changes as needed until the branch goes to master, with new scaffolding developed with that in mind.

Kathy Lussier (klussier)
Changed in evergreen:
assignee: Jennifer Pringle (jpringle-u) → nobody
Changed in evergreen:
assignee: nobody → Mike Rylander (mrylander)
Revision history for this message
Mike Rylander (mrylander) wrote :

I've been testing this today with an eye toward committing for 3.0, but I see a few issues:

 * The template uses a hard-coded id attribute, so I'm concerned about how it will act on pages with multiple org selectors in use. Maybe we can specify the popup template in some other way, like loading it from a separate file.
 * It does not always give you the dropdown when clicked. It will the first time if a default value is supplied, but not after a value is selected. I think a down-arrow button would solve this.
 * Some interfaces supply a default value to the selector, such as the vol/copy editor, but the value is not always used. This seems intermittent.
 * Related to the previous one, on some interfaces, such as the vol/copy editor again, if you click on the text box and get the popup the first time, but then click outside to leave it as it is, the value is cleared.
 * Again, with the vol/copy editor as an example, there are some additional interfaces that need work to constrain the size of the input.

Thanks, Kyle, and let me know if any of that is unclear.

Changed in evergreen:
assignee: Mike Rylander (mrylander) → nobody
Kyle Huckins (khuckins)
tags: removed: pullrequest
Revision history for this message
Kyle-huckins (kyle-huckins) wrote :

I've had a chance to take another look at this. I'm wondering if it might be worth utilizing the egBasicComboBox and deprecating egOrgSelector. The more I look at my edits and the original code, the more I think that switching over may be the way to go here - it has all the marks of what we want the selector to be able to do, how we want it to do it, and would result in less code to maintain in the long run. So I guess the question becomes: Does egOrgSelector actually need to be separate? Is there anything that it handles that egComboBox shouldn't handle? The one thing I see that could pose a problem with this approach is in displaying the hierarchy of OUs in the dropdown, but there could be more that I'm not seeing at this moment.

Revision history for this message
Joan Kranich (jkranich) wrote :

We also need the ability to type in the Main (Profile) Permission Group to access a specific group in the list. It is time consuming for staff to scroll down a list if the list is long. We have many Permission Groups which makes this difficult for staff.

Changed in evergreen:
status: New → Confirmed
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I'm setting the importance to High because it has been identified to me as critical to fix this.

Changed in evergreen:
importance: Medium → High
Dan Pearl (dpearl)
summary: - webclient: Need ability to type to selection in in some menus
+ webclient: Need ability to type to selection in some menus
Kathy Lussier (klussier)
tags: added: webstaffblocker
Revision history for this message
Kyle Huckins (khuckins) wrote :

With the increased interest in this bug, it's worth summarizing where we left off with this issue. The latest commit I've pushed for it has some design issues outlined by Mike in comment #33, which I'll copy/paste here so there's less need to scroll:

* The template uses a hard-coded id attribute, so I'm concerned about how it will act on pages with multiple org selectors in use. Maybe we can specify the popup template in some other way, like loading it from a separate file.
 * It does not always give you the dropdown when clicked. It will the first time if a default value is supplied, but not after a value is selected. I think a down-arrow button would solve this.
 * Some interfaces supply a default value to the selector, such as the vol/copy editor, but the value is not always used. This seems intermittent.
 * Related to the previous one, on some interfaces, such as the vol/copy editor again, if you click on the text box and get the popup the first time, but then click outside to leave it as it is, the value is cleared.
 * Again, with the vol/copy editor as an example, there are some additional interfaces that need work to constrain the size of the input.

It was brought up in a later comment that having this functionality would be useful in the permission group dropdown as well - my own thoughts on this would be that it might be worth splitting this out into multiple issues tackling different menus, since there's enough difference between each dropdown that that might be more of an undertaking for one issue than it needs to be.

Over the course of the issue and trying to refactor it in the past, I wasn't able to come up with a good way to leverage the code I'd written to cover some of these issues. My thoughts at that point were that we may be able to deprecate egOrgSelector and utilize egBasicComboBox, since that already has much of the functionality we want. The problem there is that displaying the hierarchy correctly may be difficult, and org-specific stuff might have to be pushed to interface-specific controllers, which might not be ideal.

Revision history for this message
Bill Erickson (berick) wrote :

Kyle, for hierarchical formatting of a combobox, if CSS won't work (it won't in a <select>), one trick is to insert unicode space characters to push labels to the right (based on tree depth), instead of regular spaces, so the browser won't collapse the values.

E.g. https://www.fileformat.info/info/unicode/char/2007/index.htm

You can copy/paste the value from the input field at:

https://www.fileformat.info/info/unicode/char/2007/browsertest.htm

If you go this route, though, you have to ensure the preceding spaces don't interfere with the type-ahead matching.

I would prefer not to change the <eg-org-selector/> interface this late in the release cycle. Maybe it's possible to use a combobox inside the org selector directive, so that references to <eg-org-selector/> won't have to change?

+1 to new LP for permission group selector.

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

Re: permission group dropdown

While allowing type to select in this dropdown is also important to implement at some point, I believe that incorporating bug 1744756 into 3.2 will alleviate most of the pain of navigating through that particular dropdown.

With that in place, typing to select in the org unit selector is definitely the top priority.

Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

Standing squarely on Kyle's shoulders, I took at stab at a different approach to the org selector. I built it atop angular bootstraps "typeahead" widget. It's a standard type-ahead input (no down arrows) which supports tab-select of the (filtered) displayed values. With a bit of hackery it also supports click-to-open regardless of the value in the selector.

Top 2 commits:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1511742-org-select-typeahead

Note the 2nd commit fixes some display issues, which cropped up as a result of changing the org selector markup. I was not able to track down any others, but they may exist.

tags: added: pullrequest
Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
milestone: none → 3.2-rc
Revision history for this message
Kyle Huckins (khuckins) wrote :

I've added and signed off on a commit that fixes some additional display issues in the Acquisitions Request list, Age Circs to Lost, and the Auto Print settings UI. Thanks Bill!

Changed in evergreen:
assignee: nobody → Kyle Huckins (khuckins)
assignee: Kyle Huckins (khuckins) → nobody
Revision history for this message
Kyle Huckins (khuckins) wrote :
Revision history for this message
Galen Charlton (gmc) wrote :

I'm taking Kyle's #41 and #42 as also being a signoff of Bill's patches, so I'm now reviewing this for potential merging to master now.

Changed in evergreen:
assignee: nobody → Galen Charlton (gmc)
Revision history for this message
Galen Charlton (gmc) wrote :

Pushed to master. Thanks, Bill and Kyle.

While testing, I noted that there is a lot of inconsistency in how we label OU selectors through the interface, but sorting that out should be the topic of a separate bug.

Changed in evergreen:
status: Confirmed → Fix Committed
assignee: Galen Charlton (gmc) → nobody
Changed in evergreen:
status: Fix Committed → Fix Released
Revision history for this message
Cesar V (cesardv) wrote :

Just noting here: It looks like since the template for the OU selector directive has the outer element being a span tag, the input field will fill it's container, and stretch the whole width of the page in some UIs.

Opened bug 1797973 with a possible way to address it.

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.