[User Story] Show loading state in search app

RESOLVED FIXED in 2.1 S3 (29aug)

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: pdol, Assigned: daleharvey)

Tracking

({feature})

unspecified
2.1 S3 (29aug)
x86
Mac OS X
feature
Dependency tree / graph

Firefox Tracking Flags

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

Details

(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 attachment)

Comment hidden (empty)
Depends on: 1039727
Target Milestone: --- → 2.1 S2 (15aug)
(Reporter)

Updated

4 years ago
User Story: (updated)
(Reporter)

Comment 1

4 years ago
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)

Updated

4 years ago
Assignee: nobody → dale
Target Milestone: 2.1 S2 (15aug) → 2.1 S3 (29aug)
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)
(Assignee)

Comment 5

4 years ago
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)
(Assignee)

Comment 7

4 years ago
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)
(Assignee)

Comment 8

4 years ago
Updated to use the building block, thanks
(Assignee)

Updated

4 years ago
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)
(Assignee)

Comment 11

4 years ago
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+
(Assignee)

Comment 13

4 years ago
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
You need to log in before you can comment on or make changes to this bug.