Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sort Pageable Analyze Tables Over Full Collection #2088

Merged
merged 2 commits into from
Jul 27, 2017

Conversation

arottersman
Copy link

Overview

  • Previously, clicking the column headers for the Point Source or Catchment Water Quality tables, would only sort the data within the visible page
  • Extract out a PageableTableBaseView that handles pagination, and overrides the bootstrap-table's onSort to sort the whole collection, and rerender the table

Connects #1594

Demo

1rkoyw9t2h

Testing Instructions

  • Pull + ./scripts/bundle.sh
  • Analyze a big AoI in the DRB (eg the schuykill huc-08)
  • Confirm the Point Source and Water Quality tables
    • still paginate properly
    • sort across the whole table on column sort for each column
    • still download
  • Site storm model the AoI and confirm the Analyze toggled view works and isn't laggy
  • For a smaller AoI confirm the point source + water quality tables still render and sort okay when not paginated
  • For an AoI outside the DRB, confirm the point source + water quality no-data tables render okay

- Previously, clicking the column headers for Point Source or Catchment Water Quality tables, would only sort the data within the visible page
- Extract out a PageableTableBaseView that handles pagination, and overrides the bootstrap-table's onSort to sort the whole collection, and rerender the table
@mmcfarland
Copy link
Contributor

Looks good, though I have one UX suggestion. When paged into the results, I think doing any sort should reset the page back to 1. The page you happen to be on won't necessarily contain any of the records it previously did or have much context for the new sort order. When I was on page 5 and sorted to see the minimum discharge, it sorted correctly but left me on page 5, so I had to scroll back to actually see the minimum charge.

@arottersman
Copy link
Author

Good point — I'll add a commit to address.

@arottersman
Copy link
Author

This is ready for another look

Copy link
Contributor

@mmcfarland mmcfarland left a comment

Choose a reason for hiding this comment

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

+1, that feels like better behavior. Just kicked off another PR build, looks like it had a transient failure.

@mmcfarland mmcfarland assigned arottersman and unassigned mmcfarland Jul 27, 2017
@arottersman arottersman merged commit becb866 into develop Jul 27, 2017
@arottersman arottersman deleted the arr/table-sorting branch July 27, 2017 17:20
@rajadain rajadain mentioned this pull request Oct 16, 2017
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.

None yet

3 participants