tpac: copy location filter missing from tpac

[Kathy Lussier]

From the list of JSPAC features missing in tpac at http://evergreen-ils.org/dokuwiki/doku.php?id=dev:opac:template-toolkit:plan.

TPAC no longer has a copy location limiter on the advanced search page allowing a user to limit to a copy location(s) when scoped to a particular org unit.

[Chauncey Montgomery (montgoc1)]

This feature was extremely useful when pulling items for schools and daycares. We would limit our searches to the picture book collection or juvenile nonfiction collection. Our work around in TPAC was to create copy location groups, but creating too many of these makes the list of locations in the OPAC quite unwieldy. Having a simple copy location limiter in the advance search page would provide more flexibility and a cleaner interface.

[Rogan Hamby (rogan-hamby)]

This should be a bug fix not a wishlist.

[Bill Erickson (berick)]

The technical challenge with this feature is that it requires Javascript to be fully functional. In short, when the user changes the search org unit, the list of locations needs to reload to reflect locations within scope of the selected org unit. Without JS, the user may search a combination of org unit and location that do not overlap, thereby leading to empty results.

So, the question is, do we only support this feature if Javascript, or more specifically, if "want_dojo" is enabled? That goes against the spirit of TPAC, but I also see no other way to fully implement the feature. Comments / alternate suggestions appreciated.

[Bill Erickson (berick)]

It's a bit clunky, but we could provide a link/button to reload the advanced search interface ("Apply New Search Location") when a new org unit is selected and JS/dojo is disabled.

[Rogan Hamby (rogan-hamby)]

It's pretty far from perfect but I think it's a reasonable compromise so we can still have a TPAC not reliant on dojo. +1

[Bill Erickson (berick)]

Note to self, data loading logic for both approaches should be implemented within the mod_perl. In the JS context, a change of org unit just magically reloads the page, which will be functionally equivalent to clicking the "apply new search location" button in the non-JS context. In both cases, we need to capture and propagate to the reloaded page any values the user has already entered into the form, prior to changing the org unit.

[Dan Wells (dbw2)]

If "the spirit of TPAC" == "avoid JavaScript even when it makes the interface noticeably better", then I personally never bought into that spirit. It is already hard, and it is only going to get harder to meet user interface expectations without *some* client-side processing of interactive elements.

For me, the main issue with JSPAC wasn't the use of JavaScript, or even Dojo, but rather an overuse of asynchronous requests coupled with a lot of custom (i.e. not uniform) code. It was hard to customize, caused hard-to-understand bugs, and strained less-than-perfect connections. On our slightly customized production instance, a single page load of a results list in JSPAC makes over 120 asynchronous requests, and that's without even interacting with the page at all. It's just the load. In TPAC with want_dojo, we are between 40 and 50. Since the vast majority of that is just loading Dojo itself, I imagine we could reduce that number, but I am not sure we really need to.

The TPAC is highly functional as-is, but I think it is time to turn the page and begin adding more JS-dependent features in a responsible manner. In any case, we at least should take a hard look at what we would gain vs what we might lose, consider our current resources, and then decide (as best we can) which path results in the most compelling and competitive catalog for Evergreen.

[Rogan Hamby (rogan-hamby)]

For the record, I don't object to using javascript to accomplish this cleanly. My users pretty much all have js capable browsers to the degree that we feel comfortable saying "you need a modern version of ie/firefox/chrome" to those who don't. But, if a debate about dojo were to sideline this I don't think it's worth it. I really wish I could multiple heats on this as it's a big material issue for my users, and I don't see how it can't be for many others.

[Rogan Hamby (rogan-hamby)]

What I neglected to type there was that if we're not going to get sidelined by the argument I would prefer the cleaner solution and just put the JS in since it is, as Dan points out, all in the load.

[Pasi Kallinen (paxed)]

+1 to using javascript ("with great power ..." &c).

[Bill Erickson (berick)]

I wholeheartedly agree, Dan. To clarify, by "spirit of the TPAC", I mean that we avoid JS when equivalent functionality can be achieved without it, not that we should never use it. It would seem in this case that we can reach near, but not quite equivalent functionality, which is why I'm angling for consensus.

So far, there are two options on the table:

1. All JS. No page reloads. Better for the vast majority of users. Offers nothing to non-JS clients.

2. Mostly non-JS, with a dash of JS to force page reload when the org unit selector changes. "apply new search location" for non-JS users. Not as smooth, but offers something for everyone.

Choose your own adventure! FWIW, I prefer #1, partially because we already have non-JS support for copy location group searching, which allows libraries to create canned searches for the most popular copy locations. (Also, if you wanted to get wacky, you could document how to search copy locations manually using the existing search grammar: foobar locations(1,2,3,4,5)).

[Rogan Hamby (rogan-hamby)]

+1 to #1

[Bill Erickson (berick)]

I've pushed a JS-only version.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1084732-tpac-copy-loc-filter

It seems to work OK. There are some design decisions I'm not thrilled with, namely the org tree stuff, but the alternatives required loading the IDL, which I'm hesitant to do if it's not necessary, since it will add more load time delays.

I've tested in Chrome and FF. Someone will need to try IE. There's a chance it will fail, because it's just awful. Bug reports appreciated.

Remember, just like in the JSPAC, you have to select an org unit where can-have-vols is true (usually a branch) before the selector will display.

[Bill Erickson (berick)]

TODO: need to fix the problem where hidden orgs cause a gap in the tree.

[Bill Erickson (berick)]

Fix force-pushed back to same branch.

[Ben Shum (bshum)]

Tested the fixed branch and it works fine for us. Tested with Firefox, Chrome, and IE10.

Going to let others take a stab at it as well, but I think we're fine with merging this feature to master.

[Rogan Hamby (rogan-hamby)]

While my gitfu failed me, I manually copied over the changes to my VM and tested and it looks good.

[Kathy Lussier (klussier)]

Just one question before the code is put in. Should the copy location selector have an aria-label for accessibility? If so, maybe the label should be "Select Copy Location."

[Pasi Kallinen (paxed)]

I haven't tested this yet, but is it possible for a copy location name to contain a double quote (")? If so, does that break the javascript aou_hash?

[Bill Erickson (berick)]

Kathy, we can certainly add an aria-label. If we're using "Select Copy Location.", do we want to change the main label (that sits above it) to "Copy Location" instead of "Shelving Location" (which is what the JSPAC used) to match?

Pasi, it's probably possible. I'll escape it.

[Kathy Lussier (klussier)]

Errrr...sorry. I guess I meant to say "Select Shelving Location." Consistency FTW!

[Bill Erickson (berick)]

Thanks. Force-pushed a rebased, squashed branch to same location which adds the aria-label and protects against "'s in org names.

[Ben Shum (bshum)]

Received confirmation that this is working with the changes berick noted. Pushed to master for happy return in 2.5. Thanks everyone!