1026007 - [Rocketbar] Notification is not dismissed after hitting Enter

Status: RESOLVED FIXED

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

Description

[Johan Lorenzo [:jlorenzo]]

As per specs: "If the user initates a search by tapping the Go button or Enter key or a search suggestion, dismiss the pop-up and don’t show it again."

Comment 1

[Johan Lorenzo [:jlorenzo]]

Do you think it is a blocker for UX?

Comment 2

[Francis Djabri [:djabber]]

Not a blocker as far as I'm concerned.

Comment 3

[Dale Harvey (:daleharvey)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/20832

Comment 4

[Kevin Grandon :kgrandon]

Comment on attachment 8443817 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20832

Clearing review for now as I think I need some more UX input on this.

1 - I don't see a go button yet for 2.0, are we going to have one?

2 - Pressing enter currently always opens and takes you to the browser now for search - making the notification go away.

Basically it seems like we should only land this if we have a go button, or if we return after hiding the notification so we don't show the browser. What do you think?

Comment 5

[Dale Harvey (:daleharvey)]

We arent going to have a go button as mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1023819#c10, the clear button replaced it

Prior to this patch if you return to the search app after the browser was opened, the notification will still be present, this was what the patch fixes

Comment 6

[Kevin Grandon :kgrandon]

(In reply to Dale Harvey (:daleharvey) from comment #5)
> We arent going to have a go button as mentioned in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1023819#c10, the clear button
> replaced it
> 
> Prior to this patch if you return to the search app after the browser was
> opened, the notification will still be present, this was what the patch fixes

Cool - original bug description didn't mention that, so I wasn't sure what I was looking for. We should be good here then, thanks!

Comment 7

[Dale Harvey (:daleharvey)]

Comment on attachment 8443817 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20832

Updated the test to deal with the default search engine change, got a kinda r+ before but want to make sure

Comment 8

[Kevin Grandon :kgrandon]

Comment on attachment 8443817 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20832

Seems to work, thanks!

Comment 9

[Kevin Grandon :kgrandon]

Landed: https://github.com/mozilla-b2g/gaia/commit/f8270bbf7fdd366f9684deb047302c6f522af117

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8443817 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20832

This is needed for the vertical homescreen.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/1e17672cdf953b9777c12bbec8ef1f6f8a5b20f3

Comment 12

[SandKing]

Hi Johan,
From your conversation, I can see "Go" button has been replaced by "Clear" button. But actually there is neither "Go" or "Clear" button displayed  on the view. Only one "Magnifier" icon displays above the SIP. Do you mean this "Magnifier" icon is "Go" button?

Comment 13

[Johan Lorenzo [:jlorenzo]]

(In reply to SandKing from comment #12)
> Do you mean this "Magnifier" icon is "Go" button?

Yes, the magnifying glass is the Go button described in the specs at the time.

Comment 14

[SandKing]

Hi Johan,
Would you please provide the detailed reproduce steps for me to verify this bug?
I'm not sure whether I use the right step to verify it.
Thanks!