Closed Bug 1041380 Opened 6 years ago Closed 6 years ago

[User Story] Show loading state in search app

Categories

(Firefox OS Graveyard :: Gaia::Search, defect)

x86
macOS
defect
Not set

Tracking

(feature-b2g:2.1, tracking-b2g:backlog)

RESOLVED FIXED
2.1 S3 (29aug)
feature-b2g 2.1
tracking-b2g backlog

People

(Reporter: pdol, Assigned: daleharvey)

References

Details

(Keywords: feature, Whiteboard: [ucid:System240], [ft:systemsfe])

User Story

As a user, I want to be made aware that results are loading in the search app when non-local results are still in the process of loading so that I can tell what is happening.

Acceptance Criteria:
1. While e.me or Marketplace results are loading in the search app, a loading indicator is displayed, per UX spec.
2. After scrolling down to load more results from the server, a loading indicator is displayed, per UX spec.
3. After results are loaded in AC1 or AC2, the loading indicator disappears.

See page 14 of v12 of Search App spec: https://mozilla.app.box.com/s/lbw2wzw3p4jvxs24k4sg
See Visual spec: https://mozilla.app.box.com/s/2iocehbfaupyc4g65lkw

Attachments

(1 file)

No description provided.
Depends on: 1039727
Target Milestone: --- → 2.1 S2 (15aug)
User Story: (updated)
Amir, is this something that you can help with over the next few days?
Flags: needinfo?(amirn)
Peter, let's continue discussion over email.
Flags: needinfo?(amirn)
Assignee: nobody → dale
Target Milestone: 2.1 S2 (15aug) → 2.1 S3 (29aug)
Attachment #8477822 - Flags: review?(kgrandon)
Comment on attachment 8477822 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/23234

Looks good, but the only thing that's weird is that the loading indicator is shown forever when the device is offline. Maybe we need to detect if the device is offline or fire off a reject() call at the right time?

Can you look into it and re-flag me for review? Thanks!
Attachment #8477822 - Flags: review?(kgrandon)
Comment on attachment 8477822 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/23234

I assumed we didnt do remote searches while offline, but we already reject the promises properly so added a handler for that, good catch, cheers
Attachment #8477822 - Flags: review?(kgrandon)
A few questions if I may, Dale

1. Did you consider using the <progress> building block? http://buildingfirefoxos.com/building-blocks/progress-and-activity.html

2. I'd like to use this in Collections
* Could it be ready for vertical alignment?
* Does the calculated position take divider elements into account?
* Would it's position update if new grid items would be added?

Thanks, Ran
Flags: needinfo?(dale)
1. I didnt notice the building block, picked up the css from the video app, but probably best to switch it, thanks

2. For the search app I just lay it out with padding below the gaia-grid, it isnt part of the grid, part of this patch is to have gaia-grid update its height as it renders so when things are added to the grid it moves below them, its only removed one remote results are shown, if you want to vertically center it seems like that would be fine too
Flags: needinfo?(dale)
Updated to use the building block, thanks
Duplicate of this bug: 1039727
Comment on attachment 8477822 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/23234

Left some comments on github. Could we try not setting the height of the grid directly? I don't think it's going to play well with the <from, to> arguments.
Attachment #8477822 - Flags: review?(kgrandon)
Comment on attachment 8477822 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/23234

Set the grid height inside the provider, as an API I think the grid should properly define its own height, but it this will have less impact and works good
Attachment #8477822 - Flags: review?(kgrandon)
Comment on attachment 8477822 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/23234

(In reply to Dale Harvey (:daleharvey) from comment #11)
> Comment on attachment 8477822 [details] [review]
> https://github.com/mozilla-b2g/gaia/pull/23234
> 
> Set the grid height inside the provider, as an API I think the grid should
> properly define its own height, but it this will have less impact and works
> good

Ok, thanks. Thinking about it again, we could potentially move this same logic into the grid, I think it's better than trying to add the height that might get out of sync. Let's revisit this if we need something similar for smart collection loading.
Attachment #8477822 - Flags: review?(kgrandon) → review+
Yeh we can follow this up if needed, as said I would prefer the grid set its own height, but this closes it out for now

https://github.com/mozilla-b2g/gaia/commit/9f79ffc8f284cda736d2ed64b45c11e9ac578e6a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.