Closed
Bug 1015296
Opened 10 years ago
Closed 10 years ago
[Ringtones] Update to use gaia-header
Categories
(Firefox OS Graveyard :: Gaia::Ringtones, defect)
Tracking
(ux-b2g:2.1)
RESOLVED
FIXED
ux-b2g | 2.1 |
People
(Reporter: yor, Assigned: wilsonpage)
References
Details
Attachments
(1 file, 1 obsolete file)
No description provided.
Blocks: gaia-header
Comment 1•10 years ago
|
||
We should probably wait until bug 960329 is fixed before working on this.
Depends on: 960329
Attachment #8431363 -
Flags: review?(wilsonpage)
Attachment #8431363 -
Flags: review?(squibblyflabbetydoo)
Assignee | ||
Comment 3•10 years ago
|
||
Comment on attachment 8431363 [details]
Pull request
Looks good, Yan's da Man!
- Need to rebase and remove `data-` prefixes
- Noticed that we need to remove `data-` from <gaia-subheader>, should that be a separate issue, or bundled with this?
- See Github comments
Attachment #8431363 -
Flags: review?(wilsonpage) → review+
Comment 4•10 years ago
|
||
Comment on attachment 8431363 [details]
Pull request
This code looks sane overall, but it's currently causing test failures, so we'll need to get those fixed before this can land.
Attachment #8431363 -
Flags: review?(squibblyflabbetydoo) → review-
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8431363 [details]
Pull request
This patch need reworking following changes in v2.0 and updated import workflow.
Attachment #8431363 -
Flags: review+
Comment on attachment 8431363 [details]
Pull request
Updated based on latest master and all tests passed.
Attachment #8431363 -
Flags: review- → review?(squibblyflabbetydoo)
Flags: needinfo?(yor)
Attachment #8431363 -
Flags: review?(squibblyflabbetydoo)
Attachment #8431363 -
Flags: review?(squibblyflabbetydoo)
Comment 8•10 years ago
|
||
Marking this bug for 2.1 since the web components Header is the one committed web component for 2.1. Other web components for 2.2 depend on Header going first. Sorry for doing this late; Hema just brought this bug to my attention today, but it reflects agreed scope and is in the agreed 2.1 plan. Just updating the flags appropriately for release tracking.
ux-b2g: --- → 2.1
Comment 9•10 years ago
|
||
This review has not moved for a couple of months. We are in sprint 3 of 2.1 and this item blocks 2.1. We need reviews to move so that these items can land *this week*, before Friday 8/22. Squib, please reassign or review ASAP (+ Hema to advise on reassignment if needed). Let me know if anything else is blocking here. Thanks!
Flags: needinfo?(squibblyflabbetydoo)
Flags: needinfo?(hkoka)
Reporter | ||
Comment 10•10 years ago
|
||
Actually Jim did review this the first time around back in May. I just resubmitted for final review last week. But yes, we need to get the review done soon if we hope to land by Friday.
Comment 11•10 years ago
|
||
Comment on attachment 8431363 [details]
Pull request
I tried the patch out and noticed a few significant problems:
1) The "+" icon doesn't show up in the "Manage Ringtones" activity.
2) The colors for the header have changed; everything seems darker now. Most importantly, this includes the "Done" label in the picker, which doesn't initially show up as disabled.
3) There appears to be a *huge* performance regression in scrolling the lists, as well as some visual glitches while scrolling (parts of the background show up as black).
Without these issues resolved, there's no way we can land this for 2.1.
Attachment #8431363 -
Flags: review?(squibblyflabbetydoo) → review-
Flags: needinfo?(squibblyflabbetydoo)
Updated•10 years ago
|
Flags: needinfo?(hkoka)
Assignee | ||
Comment 13•10 years ago
|
||
Assignee: yor → wilsonpage
Attachment #8431363 -
Attachment is obsolete: true
Attachment #8476075 -
Flags: review?(squibblyflabbetydoo)
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Jim Porter (:squib) from comment #11) > Comment on attachment 8431363 [details] > Pull request > > I tried the patch out and noticed a few significant problems: > > 1) The "+" icon doesn't show up in the "Manage Ringtones" activity. Fixed > 2) The colors for the header have changed; everything seems darker now. Colors are not tied to this patch, they come from the central gaia-theme CSS. I'm working with visual to get the exact color changes they require for v2.1. Once this is resolved all the apps will update, so this should block this patch landing. > Most importantly, this includes the "Done" label in the picker, which doesn't > initially show up as disabled. Fixed > 3) There appears to be a *huge* performance regression in scrolling the > lists, as well as some visual glitches while scrolling (parts of the > background show up as black). Fixed
Flags: needinfo?(squibblyflabbetydoo)
Comment 15•10 years ago
|
||
Comment on attachment 8476075 [details]
pull-request (master)
Ok, things look good. It seems like scrolling perf is still worse than it was pre-patch, but it's not a big issue anymore.
Attachment #8476075 -
Flags: review?(squibblyflabbetydoo) → review+
Flags: needinfo?(squibblyflabbetydoo)
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8476075 [details] pull-request (master) LANDED https://github.com/mozilla-b2g/gaia/commit/13785d9181804b095ba49444e31237d442675228
Assignee | ||
Updated•10 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•