Replace table-based display with jsTree for accessibility

Activity

CONTRIB-24 56

Keyboard shortcuts  
Summarize the review outcomes (optional)
 
#permalink

Details

Warning: no files are visible, they have all been filtered.
Participant Role Time Spent Comments Latest Comment
Author 1h 33m 24 Yes. I plan to get them all before the final commit.
Reviewer - 95% reviewed 29m 7 So when options.hasOwnProperty('onlyOwnedFolders') evalua...
Reviewer - 97% reviewed 24m 3 There are a few todo's in the different files. Remember t...
Reviewer - 91% reviewed 7m    
Patrick Haggood (deleted user)
Reviewer - 97% reviewed 25m 4 document.ready is my favorite javascript function. not.
Reviewer - 97% reviewed 46m 11 remove the comment if no needed
Reviewer - Complete 36m 7 The nesting depth in googleDriveLti.js will make it hard ...
Reviewer - 0% reviewed 0m    
Total   4h 20m 56  
#permalink

Objectives

Inspired by GOOGLE-115, the GDL tool's table-based UI is replaced by jQuery plug-in jsTree.

Gonzalo's suggestion was to use a jQuery plug-in called jsTree (see link below), which is a full-featured tree, features ARIA-compliant accessibility, and was also planned for use with the Sakai resources UI. Basically, replace all the locally-written code that displays the folder/file tree with simpler code that calls this jQuery plug-in to display it instead. This will require some moderate rewriting of the GDL UI code, but it will also simplify the project's codebase.

The code in this review is functional and covers the basic GDL UI requirements. Some of the other features (e.g. searching, adding new items to folders, etc.) have been moved to other JIRA issues and are not complete here. This code could stand some cleaning up and clarification. I hope the review comments received here will help me concentrate on what others think needs to be fixed.

Branches in review

#permalink

Issues Raised From Comments

Key Summary State Assignee
#permalink

General Comments

David Haines

The nesting depth in googleDriveLti.js will make it hard to understand the co...

The nesting depth in googleDriveLti.js will make it hard to understand the code in the future. It should be straightforward to replace some of the most nested code by separate functions for the sake of clarity.

Consider using underscore.js where appropriate and avoid rewriting utility functions.

Lance E Sloan

As a matter of fact, after I opened this review, I started working on related...

As a matter of fact, after I opened this review, I started working on related JIRA issues (e.g. the one for fixing the sorting) and I decided that it would be much clearer to move the sorting function out of the initialization one. I wasn't sure it would work, since I didn't understand the scope of "this", but it has worked so far. I plan to do the same with the other functions.

Thanks for the underscore.js tip.

/umich/google/.../css/google-drive-lti.css Changed 2
/umich/google/.../webapps/js/googleDriveLti.js Changed 43
/umich/google/.../webapps/js/utils.js Changed 1
/umich/google/.../main/webapps/jstree/ Added
Open in IDE #permalink
/umich/google/.../webapps/jstree/themes/ Added
Open in IDE #permalink
/umich/google/.../pages/link-google-drive.jsp Changed 8
/umich/google/.../pages/show-google-drive.jsp Changed
/umich/google/.../webapps/view/root.jsp Changed

Review updated: Reload | Ignore | Collapse

You cannot reload the review while writing a comment.

Create issue

X
Assign To Me

Log time against