Skip to content

Conversation

@andy-armstrong
Copy link
Contributor

Added sorting to the Files & Uploads page. This is built upon the changes in andy/asset-pagination which will be delivered first.

@cahrens @singingwolfboy @frrrances Please review.

andy-armstrong and others added 15 commits December 30, 2013 15:39
These changes represent the initial implementation of STUD-813
which adds pagination to the Files and Uploads page.

The commit consists of the following logical changes:
 - a REST API has been implemented for a course's assets
 - the page itself now fetches the assets client-side
 - the Backbone.Paginator library is used to support pagination
 - the AssetCollection has been refactored to extend
   Backbone.Paginator.requestPager so that it can be paged
 - an abstract PagingView class has been added to generalize
   the communication with a paging REST API
 - the AssetsView has been reimplemented to extend PagingView
 - two new child views have been added:
   - PagingHeader: the paging controls above the list of assets
   - PagingFooter: the paging controls below the assets

Some areas that need further improvement:
 - the page size is hard-coded to 5 to make it easier to reproduce
 - the 'format=json' option to the REST API is non-standard
 - there are no tests (they will be coming in my next commit)

Note that this commit will not be delivered as is, but is provided
for pre-review purposes.
Improve UX based on feedback from Frances
Fix unit tests
Add some testing around pagination of JSON requests
Updated asset view Jasmine tests to work with new AJAX model
Refactored my JavaScript code to be more testable
Start of sorting implementation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of sort_direction=descending, can we do sort_dir=desc? It seems just as clear, and it's a lot shorter. (If you think it's less clear, please let me know!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to direction=asc/desc as we discussed, since that's what GitHub uses.

@singingwolfboy
Copy link
Contributor

Your Javascript tests are amazing! You are my hero. 😍

@cahrens
Copy link

cahrens commented Jan 3, 2014

I'm not seeing any ascending/descending indication. Personally, I also think the indication of which columns are sortable (and which is currently selected) is too subtle. Does this have all of @frrrances styling?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the method documentation to state what the valid query parameters are now.

@andy-armstrong
Copy link
Contributor Author

@cahrens, this is the design from @frrrances. She double checked it with me on Tuesday. I like the simple design, but you should discuss the choices with her.

- Added missing Python tests
- Added some extra JavaScript tests
- Removed unused code
- Updated doc strings
@andy-armstrong
Copy link
Contributor Author

Rebased to master, so submitted a new pull request: https://github.com/edx/edx-platform/pull/2086

@andy-armstrong andy-armstrong deleted the andy/asset-sorting branch January 6, 2014 17:03
shimulch pushed a commit to open-craft/openedx-platform that referenced this pull request Jan 26, 2021
[YONK-1866] Merge master changes into rebase-juniper(Phase 3).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants