Closed Bug 1088389 Opened 7 years ago Closed 7 years ago

[Search] Extra divider shown when opening search

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S7 (24Oct)
Tracking Status
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: kgrandon, Assigned: kgrandon)

Details

(Whiteboard: [systemsfe][2.1-bug-bash])

Attachments

(3 files)

Sometimes when opening the search app and performing a search an unsightly extra divider is shown. We should remove it.
Attached file Github pull request
Dale - could you take a look at this one?
Attachment #8510709 - Flags: review?(dale)
Comment on attachment 8510709 [details] [review]
Github pull request

What do you think about testing this? Its obviously testable but I do feel like this is hitting the area where a full on integration test may not be worth it
Attachment #8510709 - Flags: review?(dale) → review+
(In reply to Dale Harvey (:daleharvey) from comment #2)
> Comment on attachment 8510709 [details] [review]
> Github pull request
> 
> What do you think about testing this? Its obviously testable but I do feel
> like this is hitting the area where a full on integration test may not be
> worth it

Yeah, I'm not sure about this yet. On one hand it's a configuration change that we could easily write a unit test for, but that wouldn't provide much value. An integration test *may* be doable, but I agree I'm not sure if it's worth the effort as it's a configuration change.

Since we're now actively triaging which bugs need tests, let's be sure to triage this one in the future.
In master: https://github.com/mozilla-b2g/gaia/commit/1abd35240d27ef0630d98be7b3f08915b10b120c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment on attachment 8510709 [details] [review]
Github pull request

This is a tiny patch that provides a bit of polish by changing some grid configuration logic. I would like to uplift this as it's low risk, moderate reward.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Rocketbar implementation.
[User impact] if declined: Less than ideal UX when starting searching rocketbar due to an unsighly divider.
[Testing completed]: Manual testing. We are currently discussing how to test this in CI. It's quite difficult due to only happening on a resize event, then going away.
[Risk to taking this patch] (and alternatives if risky): Low risk - single line config change.
[String changes made]: None.
Attachment #8510709 - Flags: approval-gaia-v2.1?(fabrice)
Target Milestone: --- → 2.1 S7 (24Oct)
Whiteboard: [systemsfe][2.1-FC-bug-bash] → [2.1-bug-bash]
Can we get screenshots of before/after?
Flags: needinfo?(dale)
(In reply to Kevin Grandon :kgrandon from comment #5)
> Comment on attachment 8510709 [details] [review]
> Github pull request
> 
> This is a tiny patch that provides a bit of polish by changing some grid
> configuration logic. I would like to uplift this as it's low risk, moderate
> reward.
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #): Rocketbar implementation.
> [User impact] if declined: Less than ideal UX when starting searching
> rocketbar due to an unsighly divider.
> [Testing completed]: Manual testing. We are currently discussing how to test
> this in CI. It's quite difficult due to only happening on a resize event,
> then going away.
> [Risk to taking this patch] (and alternatives if risky): Low risk - single
> line config change.
> [String changes made]: None.

i dont think this should be uplifted for 2.1   given its hard to reproduce, and seems edge casey. plus its recoverable.
(In reply to Tony Chung [:tchung] from comment #7)
> i dont think this should be uplifted for 2.1   given its hard to reproduce,
> and seems edge casey. plus its recoverable.

It's actually very easy to reproduce, and I'll try to upload a video shortly. Visual blemishes like this are hugely detrimental to the UX of the phone and should be considered a blocker imo.
Attached video Video of problem
Here is a video of the problem. You can see the white divider shortly after a character is pressed.
Provided by Kevin
Flags: needinfo?(dale)
Comment on attachment 8510709 [details] [review]
Github pull request

Not bad enough.
Attachment #8510709 - Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1-
My thoughts are that since you can see this thing *every* time you open the rocketbar, and it's the new showcase feature of 2.1, we should consider this a blocker.

Francis - do you have any thoughts from a UX perspective?
Flags: needinfo?(fdjabri)
I agree with Kevin. This is more than just polish, this is confusing UI. It will provide constant reminders to the user that the product is poor quality. Given that this is such a leading feature, I feel we really need to be doing better here for 2.1.
Flags: needinfo?(fdjabri)
[Blocking Requested - why for this release]: Requesting blocking based on comment 13.
blocking-b2g: --- → 2.1?
This should block. These are, time and time again, the things we get hammered on in the market and in product reviews. If we can't ship a feature without the expected degree of consumer market functionality and polish, it shouldn't ship.  

This is not polish: it looks broken and, due to its position, it implies that there's something there (suggested search results) that aren't appearing or fully "rezzing."
Whiteboard: [2.1-bug-bash] → [systemsfe][2.1-bug-bash]
:bajaj and fabrice...can you please see team's comments on blocking?
Flags: needinfo?(fabrice)
Flags: needinfo?(bbajaj)
Comment on attachment 8510709 [details] [review]
Github pull request

Approving by popular demand. But how was that found so late if that's a prominent feature?
Flags: needinfo?(fabrice)
Attachment #8510709 - Flags: approval-gaia-v2.1- → approval-gaia-v2.1+
Thanks Fabrice. In v2.1: https://github.com/mozilla-b2g/gaia/commit/0e332da37ad3d12faa78bc0d20d95735e3b5194d

I agree we caught this way too late. Our team is taking steps to address this for the future including daily engineering quality runs. Hopefully we'll catch things like this earlier.
I don't think we need to make the argument to block here as it's been uplifted.
blocking-b2g: 2.1? → ---
Flags: needinfo?(bbajaj)
Attached video Verify_Video_Flame.MP4
This issue has been verified successfully on Flame 2.1.
See attachment: Verify_Video_Flame.MP4
Reproducing rate: 0/20

Flame v2.1 version:
Gaia-Rev        1bdd49770e2cb7a7321e6202c9bf036ab5d8f200
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/db893274d9a6
Build-ID        20141125001201
Version         34.0

Flame 2.2 version:
Gaia-Rev        824a61cccec4c69be9a86ad5cb629a1f61fa142f
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/acde07cb4e4d
Build-ID        20141125040209
Version         36.0a1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.