Skip to content

Conversation

@hmoco
Copy link
Contributor

@hmoco hmoco commented Mar 21, 2018

Purpose

Improve the loading experience between request as users filter/sort their projects on dashboard.

Summary of Changes

  • Searching (filtering) now introduces a new loader, inside the input bar, when a filter is being requested
  • Searching and sorting no longer replace current items with a loading icon. Instead, the current elements are less opaque and unable to be interacted with for the duration of the request.
    mar-21-2018 13-32-05

Ticket

https://openscience.atlassian.net/browse/EMB-168

Reviewer Checklist

  • meets requirements
  • easy to understand
  • DRY
  • testable and includes test(s)
  • changes described in CHANGELOG.md

@coveralls
Copy link

coveralls commented Mar 21, 2018

Coverage Status

Coverage decreased (-0.9%) to 21.122% when pulling e750624 on hmoco:feature/loader into 6006989 on CenterForOpenScience:develop.

findNodes: task(function* (this: Dashboard, more?: boolean) {
findNodes: task(function* (this: Dashboard, more?: boolean, search?: boolean) {
const indicatorProperty = `loading${more ? 'More' : ''}`;
if (search) {
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it make more sense to move this to the filterNodes task? I.e. set it to true before the yield then false after the yield.

@@ -1,3 +1,7 @@
<div class='ball-scale ball-light LoadingIndicator'>
<div class='ball-{{if pulse "pulse" "scale"}} {{if blue "ball-scale-blue"}} {{if dark "ball-dark" "ball-light"}} LoadingIndicator'>
Copy link
Contributor

Choose a reason for hiding this comment

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

Drive-by: Per product consultation, remove this, and when no products are found and loading, change the "No projects found" to either be the normal loading pulser or have it say, "Loading…". The pulser is better because a) it should be there already; and b) no extra text.

yield timeout(500);
this.setProperties({ filter });
yield this.get('findNodes').perform();
yield this.get('findNodes').perform(false, true);
Copy link
Member

Choose a reason for hiding this comment

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

What is the second parameter to perform for?

Copy link
Member

@jamescdavis jamescdavis left a comment

Choose a reason for hiding this comment

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

💯

@jamescdavis jamescdavis merged commit 0460519 into CenterForOpenScience:develop Mar 22, 2018
@jamescdavis jamescdavis added this to the 0.3.0 milestone May 7, 2019
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.

5 participants