[Music][NGA] Improve performance of Home view

REOPENED
Unassigned

Status

Firefox OS
Gaia::Music
REOPENED
3 years ago
2 years ago

People

(Reporter: wilsonpage, Unassigned)

Tracking

unspecified
FxOS-S8 (02Oct)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Reporter)

Updated

3 years ago
Assignee: nobody → wilsonpage
Created attachment 8652257 [details] [review]
[gaia] wilsonpage:1198192 > mozilla-b2g:master
(Reporter)

Comment 2

3 years ago
Comment on attachment 8652257 [details] [review]
[gaia] wilsonpage:1198192 > mozilla-b2g:master

Based on top of bug 1196414
Attachment #8652257 - Flags: review?(jdarcangelo)
Comment on attachment 8652257 [details] [review]
[gaia] wilsonpage:1198192 > mozilla-b2g:master

Left some comments on the PR, mostly regarding coding style and maintaining consistency. Please address them before landing. Overall, this patch looks ok.
Attachment #8652257 - Flags: review?(jdarcangelo) → review+
(Reporter)

Comment 4

3 years ago
Addressed comments and waiting for green.
(Reporter)

Updated

3 years ago
Keywords: checkin-needed
https://github.com/mozilla-b2g/gaia/commit/5dfcd9deb2fcdab894ee9cae2d31da9bac732df5
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S7 (18Sep)
Looks like there was a merge failure here. Fixing in Bug 1203212.
Carsten:

It looks like there were some git merge failures, possibly from rebasing. This patch needs backed out ASAP. It is causing Gij test failures along with some other issues.
Status: RESOLVED → REOPENED
Flags: needinfo?(cbook)
Resolution: FIXED → ---

Updated

3 years ago
Flags: needinfo?(cbook)
(Reporter)

Comment 9

3 years ago
Apologies for the time wasting. A last minute merge conflict broke the js/view.js module. Fixed and pushed. I'm not really sure why it was landed if there were test failures.
I think the test failure looked a lot like an intermittent orange we have seen, hence were wrongfully ignored. Not certain though.
(Reporter)

Comment 11

3 years ago
Comment on attachment 8652257 [details] [review]
[gaia] wilsonpage:1198192 > mozilla-b2g:master

- Decreased default album artwork jpg compression
- Restored scroll position to hide <music-search> and prevent it from blocking the list content from receiving 'click's (probably a bug).
Attachment #8652257 - Flags: review+ → review?(jdarcangelo)

Updated

3 years ago
Target Milestone: FxOS-S7 (18Sep) → FxOS-S8 (02Oct)
(Reporter)

Comment 12

3 years ago
Justin is there anything blocking us from landing this?
Flags: needinfo?(jdarcangelo)
(In reply to Wilson Page [:wilsonpage] from comment #12)
> Justin is there anything blocking us from landing this?

Let's wait until we're feature complete before we make any optimizations.
Flags: needinfo?(jdarcangelo)
I'm going to take this bug over to implement lazy album art retrieval for the Home (tiles) view. Also, most of the stuff in Wilson's patch has already landed in other patches, but some of the CSS related to the tiles in his patch still needs to land.
Assignee: wilsonpage → jdarcangelo
(In reply to Justin D'Arcangelo [:justindarc] from comment #14)
> I'm going to take this bug over to implement lazy album art retrieval for
> the Home (tiles) view. Also, most of the stuff in Wilson's patch has already
> landed in other patches, but some of the CSS related to the tiles in his
> patch still needs to land.

It could either be that or maybe related to Bug 1209716 which is about an album art race condition.
(In reply to Justin D'Arcangelo [:justindarc] from comment #15)
> (In reply to Justin D'Arcangelo [:justindarc] from comment #14)
> > I'm going to take this bug over to implement lazy album art retrieval for
> > the Home (tiles) view. Also, most of the stuff in Wilson's patch has already
> > landed in other patches, but some of the CSS related to the tiles in his
> > patch still needs to land.
> 
> It could either be that or maybe related to Bug 1209716 which is about an
> album art race condition.

Disregard this. Comment intended for another bug.
Comment on attachment 8652257 [details] [review]
[gaia] wilsonpage:1198192 > mozilla-b2g:master

Clearing r? flag. I believe we have landed other optimizations to the Home view already that render this patch obsolete.
Attachment #8652257 - Flags: review?(jdarcangelo)
Not working on this. Unassigning myself.
Assignee: jdarcangelo → nobody
You need to log in before you can comment on or make changes to this bug.