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 tiles on display rather than on receipt #445

Merged
merged 4 commits into from Jul 21, 2016
Merged

Conversation

lennym
Copy link
Contributor

@lennym lennym commented Jul 21, 2016

This removes the tile sorting algorithm from being triggered by the receipt of tiles over the websocket to being done as part of the pull of search results from the stack to be rendered. This means that we can sort based on a larger set of data and with ranking potentially available.

Each time there is a call to load more items into the feed, the section of the result stack which is currently not displayed is sorted according to its ranking data (provided by lambda-ian-rankin). The top pageSize results are then displayed (pageSize currently defaults to 5).

On first load of a new search results are loaded into the feed as follows:

  • loads the first 10 results as soon as they are available (configurable via initialPageSize prop)
  • if 10 results are not found within 3 seconds then show whatever is available
  • if 1s passes with no results received, and 10 are not yet available then show whatever is available

The loading is also simplified by removing the notion of page number, in favour of just loading "more" results whenever they are requested. This is equivalent to the old behaviour anyway as the page number was always incremented on loading of more results.

This removes the tile sorting algorithm from being triggered by the receipt of tiles over the websocket to being done as part of the pull of search results from the stack to be rendered. This means that we can sort based on a larger set of data and with ranking potentially available.

Each time there is a call to load more items into the feed, the section of the result stack which is currently not displayed is sorted according to its ranking data (provided by `lambda-ian-rankin`). The top `pageSize` results are then displayed (`pageSize` currently defaults to 5).

On first load of a new search results are loaded into the feed as follows:

* loads the first 10 results as soon as they are available (configurable via `initialPageSize` prop)
* if 10 results are not found within 3 seconds then show whatever is available
* if 1s passes with no results received, and 10 are not yet available then show whatever is available

The loading is also simplified by removing the notion of page number, in favour of just loading "more" results whenever they are requested. This is equivalent to the old behaviour anyway as the page number was always incremented on loading of more results.
@tomgco
Copy link
Contributor

tomgco commented Jul 21, 2016

Code looks fine, need to fix the tests and let someone more familiar with how the react app to confirm and merge.

Ensure that the result pausing timeout is always reset while there are still not yet enough results to render a full set.

Use existing search result timeout to trigger a flush of buffered results to the page
Also adds new tests to cover additional new functionality
@lennym lennym changed the title WIP - Sort tiles on display rather than on receipt Sort tiles on display rather than on receipt Jul 21, 2016
@nelsonic
Copy link
Contributor

LGTM

@nelsonic nelsonic merged commit d5a6203 into master Jul 21, 2016
@nelsonic nelsonic deleted the feature/tile-sorting branch July 21, 2016 15:13
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