Closed
Bug 1090562
Opened 10 years ago
Closed 10 years ago
[Rocketbar] Transitions are missing
Categories
(Firefox OS Graveyard :: Gaia::System::Browser Chrome, defect)
Tracking
(blocking-b2g:2.1+, b2g-v2.1 verified, b2g-v2.2 verified)
People
(Reporter: kgrandon, Assigned: kgrandon)
References
Details
(Keywords: regression, Whiteboard: [systemsfe])
Attachments
(3 files)
It appears that at some point we lost the rocketbar transitions that we're supposed to see entering the search app. This can be quite confusing for a user as you can see through the backdrop and see two search bars. It also looks quite bad for a new feature. STR: From an application with a minimized rocketbar, tap to enter search. Expected result: The rocketbar should expand into the full size. Actual result: The transition is missing and instead the search app immediately comes into focus.
Assignee | ||
Comment 1•10 years ago
|
||
I did a quick bisection, and here is the window I found. I'm not 100% sure it's accurate due to the overlay changes we made half-way through the release, but we should investigate. git bisect start git bisect bad fede7e11f3786f8944ec5e6b74d8c30f5790c326 git bisect good f33f82d7d97c36a1ff392ac41132319c2a0bf666 git bisect bad Bisecting: 908 revisions left to test after this (roughly 10 steps) [3ef7886e15f2be9ea6240e09335ab3a746858c41] Merge pull request #23542 from asutherland/bug1060922-email-sig-upgrade-null git bisect bad Bisecting: 453 revisions left to test after this (roughly 9 steps) [a25ae14dbd2fe3e25144a7064efc0cc4e31042b8] Merge pull request #22075 from empoalp/bug_976678 git bisect good Bisecting: 226 revisions left to test after this (roughly 8 steps) [85cd1af363fc0dc3d38adff6ef3e1c6425dc6583] Merge pull request #23320 from wilsonpage/1015292 git bisect bad Bisecting: 113 revisions left to test after this (roughly 7 steps) [c4284d24db251582731424617d268394b9045310] Merge pull request #23332 from sfoster/close-apps-task-manager-bug-1047143 git bisect good Bisecting: 57 revisions left to test after this (roughly 6 steps) [5dbbd4ee09de929d6c388d19f128c902ac7d108f] Merge pull request #23246 from karanluthra/1057798-clean-up-mozl10n-api-in-email git bisect bad Bisecting: 28 revisions left to test after this (roughly 5 steps) [acc1f9f1d7e37a3eb4dc818fc71af0e244a66bbc] Merge pull request #23347 from mnjul/revert_1024298 git bisect good Bisecting: 14 revisions left to test after this (roughly 4 steps) [68873f26ac29ef24b5ea3104c1b4574424277212] Merge pull request #23368 from yurenju/fix-space-for-profile-rule git bisect good Bisecting: 7 revisions left to test after this (roughly 3 steps) [7f2100f5aa4fc2fd2dc4682ac722f29337f0d54e] Merge pull request #23378 from KevinGrandon/bug_1052503_progress_indicator_steps git bisect bad Bisecting: 2 revisions left to test after this (roughly 2 steps) [d281d90c3679d513627e17c0f4dcf9e895c9b47d] Merge pull request #23351 from mozfreddyb/bug-1056882 git bisect bad Bisecting: 1 revision left to test after this (roughly 1 step) [3ffb2e095dd36a700fe7119f317c814be822f0a6] Merge pull request #23338 from KevinGrandon/bug_1054521_retain_search_input_apps git bisect bad Bisecting: 0 revisions left to test after this (roughly 0 steps) [1b33233dc9e579af45fe42252be51af3ca2d2fa7] Bug 1054521 - [Rocketbar] Persist search term for non-browser launches r=benfrancis git bisect bad 1b33233dc9e579af45fe42252be51af3ca2d2fa7 is the first bad commit commit 1b33233dc9e579af45fe42252be51af3ca2d2fa7 Author: Kevin Grandon <kevingrandon@yahoo.com> Date: Tue Aug 26 17:53:06 2014 -0700 Bug 1054521 - [Rocketbar] Persist search term for non-browser launches r=benfrancis
Blocks: 1054521
Whiteboard: [systemsfe]
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → kgrandon
Assignee | ||
Comment 3•10 years ago
|
||
[Blocking Requested - why for this release]: Seems this is a pretty ugly, especially for a 2.1 showcase feature. I think we need to fix this.
Status: NEW → ASSIGNED
blocking-b2g: --- → 2.1?
Assignee | ||
Comment 4•10 years ago
|
||
Here's a WIP - unfortunately this has some other strange effects with the statusbar icons. For some reason they aren't shown again when exiting rocketbar - I need to investigate why.
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8513174 [details] [review] Pull request - add rocketbar animations again Etienne - seems that I've broken the rocketbar transition before. I noticed that this might conflict with bug 1071114, so that patch is included here. In order for the statusbar icons to properly update, we need to fire the chromecollapsed/expanded events in more places, so this moves that logic. We now always listen for transition events, so the statusbar is updated with the proper width. If we do go with something like this, then I'll go ahead and remove the scroll safety timeout tests that I see you added as well.
Attachment #8513174 -
Attachment description: WIP - pull request, missing tests → Pull request - add rocketbar animations again
Attachment #8513174 -
Flags: review?(etienne)
Assignee | ||
Comment 7•10 years ago
|
||
Francis - can we get some UX opinion on whether or not this is a blocker? Being that BrowserOS is one of the primary 2.1 features, I don't think we should consider shipping with something this bad.
Flags: needinfo?(fdjabri)
Comment 8•10 years ago
|
||
I totally agree that this should be a blocker. As Kevin says, this is a showcase feature and it currently looks broken. The whole idea behind the rocket bar is that it should be a continuous object between different states, so that the user understands that it's the same thing everywhere they see it. Showing two rocket bars at the same time completely messes with this and could be very confusing. Something like this will badly effect the overall perceptions of quality of the product.
Flags: needinfo?(fdjabri)
Comment 9•10 years ago
|
||
Agree this is a blocker. Kevin, do you think you'll have a fix in time for 2.1?
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Stephany Wilkes from comment #9) > Agree this is a blocker. Kevin, do you think you'll have a fix in time for > 2.1? Yes, we should be able to get a fix in this week.
Comment 11•10 years ago
|
||
Broken feature.
blocking-b2g: 2.1? → 2.1+
Target Milestone: --- → 2.1 S8 (7Nov)
Comment 12•10 years ago
|
||
Comment on attachment 8513174 [details] [review] Pull request - add rocketbar animations again Oh, I saw it long ago and I thought it was a UX decision... should have double-checked :) Anyway, glad we're putting it back in! Love the app_chrome.js change, some comments on the rocketbar.js change. Also, when you open the rocketbar from a fullscreen app we have the same issue with missing border-radius during the transition. So we need the same fix than [1] for fullscreen apps (but with a black background). And I can see the reload button when opening the rocketbar from the calendar (I think they have location changes at launch). [1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/chrome/chrome.css#L127-129 > If we do go with something like this, then I'll go ahead and remove the > scroll safety timeout tests that I see you added as well. As long as we keep the one about not publishing anything when a transitionend for the controls bubbles.
Attachment #8513174 -
Flags: review?(etienne)
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8513174 [details] [review] Pull request - add rocketbar animations again Hey Etienne - could you take a look? This does unfortunately not fix fullscreen apps 100% yet, but since we're running out of time, I suggest landing this without fullscreen apps being 100% supported, and we can fix those later. Let me know what you think. Thanks!
Attachment #8513174 -
Flags: review?(etienne)
Comment 15•10 years ago
|
||
Comment on attachment 8513174 [details] [review] Pull request - add rocketbar animations again Comments on github, tests and a small logic error. Enough to warrant a last review round but should be quick :)
Attachment #8513174 -
Flags: review?(etienne)
Assignee | ||
Comment 16•10 years ago
|
||
Comment on attachment 8513174 [details] [review] Pull request - add rocketbar animations again Hey Etienne - thanks for the great review. I've fixed the logic, added an app chrome test, and restored the callOrder tests. Let me know what you think.
Attachment #8513174 -
Flags: review?(etienne)
Comment 17•10 years ago
|
||
Comment on attachment 8513174 [details] [review] Pull request - add rocketbar animations again Awesome, thanks!
Attachment #8513174 -
Flags: review?(etienne) → review+
Assignee | ||
Comment 18•10 years ago
|
||
In master: https://github.com/mozilla-b2g/gaia/commit/311d7dee72b0dcadc1e0911985bf774edfbfeea4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → fixed
Resolution: --- → FIXED
Assignee | ||
Comment 19•10 years ago
|
||
Comment on attachment 8513174 [details] [review] Pull request - add rocketbar animations again [Approval Request Comment] [Bug caused by] (feature/regressing bug #): Feature implementation. [User impact] if declined: Users will have a poor experience using the rocketbar, a new 2.1 feature. [Testing completed]: Manual and unit tests. [Risk to taking this patch] (and alternatives if risky): Fairly low risk as it's already broken. [String changes made]: None.
Attachment #8513174 -
Flags: approval-gaia-v2.1?(bbajaj)
Updated•10 years ago
|
Attachment #8513174 -
Flags: approval-gaia-v2.1?(bbajaj) → approval-gaia-v2.1+
Assignee | ||
Comment 20•10 years ago
|
||
Seems this needs a branch patch. I'm working on one now.
Keywords: branch-patch-needed
Assignee | ||
Comment 21•10 years ago
|
||
Assignee | ||
Comment 22•10 years ago
|
||
In v2.1: https://github.com/mozilla-b2g/gaia/commit/746e26d5f6298b965e8e95e885884bd6dc82ae67
Keywords: branch-patch-needed
Comment 23•10 years ago
|
||
This issue is verified fixed on Flame 2.1 and 2.2. Result: The rocket bar expands with the animation properly when the user taps the minimized rocket bar. Tested with Dialer, Messages, Contacts, Settings, Email, and Calendar app. Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash) BuildID: 20141105001204 Gaia: 154da5e17029a51002d5d9b7df39563d509edde6 Gecko: 3b0c3580a58d Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 34.0 (2.1) Firmware Version: v188 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash) BuildID: 20141105040206 Gaia: 7c9e7cabbde941b976e0e40a3a1d94e21aa9c5e9 Gecko: 62990ec7ad78 Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 36.0a1 (2.2) Firmware Version: v188 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in
before you can comment on or make changes to this bug.
Description
•