jQuery support for the public catalog

Bug #1642086 reported by Kathy Lussier
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

As discussed at the 2016 hack-a-way, we would like to provide some jQuery support in the public catalog, as long as we adhere to the following guidelines for jQuery usage:

- Users should be able to perform tasks without turning JavaScript on; it should just enhance the experience
- We should look to CSS or non-JS approaches first to implement functionality
- We should establish a high bar for adding jQuery UI modules to avoid proliferation

We already have some jQuery support in the web client. I talked to Bill Erickson about how we make this support available for the public catalog. We were thinking we would need to:

- move Gruntfile.js, bower.json and package.json out of the staff directory and into some other directory that can be used by both the staff client and the public catalog (I called it 'common' in my implementation)

- Update paths where necessary in Gruntfile.js

I worked on a branch at the hack-a-way that does the above. I'll dust it off this week and post what I have.

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

Three months later, I've finally dusted off the branch I worked on at the hack-a-way. I have a working branch with my first attempt to move files out of the staff directory and into a common directory where files can be used by both the staff client and the public catalog.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/kmlussier/lp1642086-move-jquery

With this branch, I was able to build the web client, log in, and perform some basic transactions. I'm posting the branch to get more eyes on it.

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

Did a quick patch review and test. I was able to build dependencies and run 'grunt test'. Staff client source HTML after install looked as expected, loading angular, etc. bits from the common directory.

I pushed a secondary commit to move README.install to common/INSTALL.adoc. (I still find this file very useful -- having it where it's needed).

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/berick/lp1642086-move-jquery

I have not pushed sign-off's yet, more testing needed.

Kathy Lussier (klussier)
tags: added: pullrequest
Changed in evergreen:
milestone: none → 3.0-alpha
Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

Rebased branch at http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp1642086-move-jquery

I ran into some conflicts that didn't seem too bad to resolve so I took a stab at it. I haven't tested it yet though, so this isn't a signoff. I'll try to test when I get a chance.

Josh

Changed in evergreen:
assignee: nobody → Mike Rylander (mrylander)
Revision history for this message
Mike Rylander (mrylander) wrote :

Kathy,

Would you be OK with /only/ moving jquery to the common area? ISTM that keeping the staff code separated (unless and until we move to angular for the OPAC) would create less churn and should have the same effect. If so, I'll work up a branch.

Or am I missing the need for angular et al to move?

Thanks!

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

Thanks for looking at this Mike!

To be honest, my memory is fuzzy on why we chose to move over the files that we did. I did the bulk of the work on this branch at the November hack-a-way after sitting down with Bill Erickson to look at the files. I mostly was following his guidance. I recall that we started with a smaller set of files, but then chose to move more in the end, but I can't remember why.

I have no objections, but you might want to run the question by him in case there is a potential issue I'm forgetting.

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

IIRC the great migration centered on having the node/npm build system live in the common directory, moving files out to sub-projects as needed, as opposed to building everything within /staff/ and copying files up and out to other projects. IOW, it made less conceptual sense to build files the TPAC depends on from within the /staff/ dir.

I also expect the desire to use other bits of JS and/or Bootstrap outside of the staff client will follow pretty quickly behind jquery.

Having said that, though, I have no objections to a less dramatic transition if that helps keep things moving.

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

Thanks, Bill. With all the topic branches for 3.0 that touch the npm/grunt stuff, I feel pretty strongly that it would keep things move. I'll propose a branch here soon...

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

Here's a proposal branch to at least see the difference in interruption to topic branches:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/miker/thin-jquery-move

Bill Erickson (berick)
Changed in evergreen:
assignee: Mike Rylander (mrylander) → Bill Erickson (berick)
status: New → Confirmed
Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Mike. Branch reviewed, follow up branch pushed:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1642086-tpac-jquery-review

1. Minor merge conflict repair
2. TPAC JS path thinko repair
3. Added Open-ILS/web/js/ui/default/common/ to .gitignore (for now anyway)
4. Added jquery to karma dependencies. Not strictly required, but seemed like a good opportunity to improve consistency between UI and test runner. Tests still work.
5. sign-off to Mike's commit.

I removed all build directories (repo and installed), rebuilt everything, ran tests, confirmed webby and tpac (when configured) are able to load jquery.

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

I've merged this to master with a minor adjustment to the .gitignore file and a followup commit to adjust the offline resources needed by UpUp.

Thanks, Kathy and Bill!

Changed in evergreen:
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  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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