Closed Bug 898106 Opened 7 years ago Closed 6 years ago

Work - Dim screen when autocomplete popup shows up.

Categories

(Firefox for Metro Graveyard :: Browser, defect, P2)

All
Windows 8.1
defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: mbrubeck, Assigned: rsilveira)

References

Details

(Whiteboard: [preview] feature=work)

Attachments

(2 files, 3 obsolete files)

Bug 883390 made the auto-complete popup cover only half the screen when in full view.  But when the start page is visible, the autocomplete results should completely replace the start page, rather than covering just half of it (which gives a confusing grids-on-grids experienc).  From IRC:

<jwilde> question: how should start screen and autocomplete interact? like, if you're on the start screen and the autocomplete popup appears, what should the start screen do?

<yuan> We don't show auto-complete screen on top of Start screen. When user types in the URL bar, the results replaces the top sites, bookmarks, etc. Just like what we already have. Does it make sense?
Whiteboard: feature=work → feature=work [preview]
Assignee: mbrubeck → jmathies
Blocks: 850737
No longer blocks: 845152
Blocks: 831910
No longer blocks: 850737
Assignee: jmathies → nobody
Whiteboard: feature=work [preview] → feature=work [preview-triage]
Whiteboard: feature=work [preview-triage] → [preview-triage] feature=work
Priority: -- → P2
Whiteboard: [preview-triage] feature=work → [preview] feature=work
As discussed at monday's meeting, we either need to dim/grey out the start page or make the autocomplete full height. Asa would prefer the former
Attached patch 898106.patch (obsolete) — Splinter Review
WIP
Grays out start screen. It's still active though, will post one where clicking on grayed out area will dismiss autocomplete.
Assignee: nobody → rsilveira
Attached patch 898106.patch (obsolete) — Splinter Review
Now clicking on a tile over the grayed out start screen will just dismiss autocomplete.

We should consider having that on regular pages too. It makes it easier to dismiss autocomplete without accidentally clicking a link.
Attachment #801881 - Attachment is obsolete: true
Attachment #802712 - Flags: review?(mbrubeck)
Attached image AutocompleteGray.png
Do you think we should add the dim panel to regular sites as well? I like that it makes it easier to dismiss autocomplete without accidentally clicking a link. There is the X on the nav bar too, but it's barely visible and I feel it's quite natural to just click/tap on empty space to dismiss. Besides, if the site bg color is similar, it might look weird.

Does the dim panel color look right?
Flags: needinfo?(ywang)
I think we should dim the content for autocomplete on sites and Firefo Start. I think this  color is a fine starting point but defer to Shorlander for final call there.
Attached patch 898106.patch (obsolete) — Splinter Review
Changed the CSS to dim regular content too.
Attachment #802712 - Attachment is obsolete: true
Attachment #802712 - Flags: review?(mbrubeck)
Attachment #803192 - Flags: review?(mbrubeck)
Dimming the panel on Start Page works, I think.

Rodrigo, we should add some padding on the left hand side of the auto-complete search results. The search results and title should be aligned at the same level with the URL field. See shorlander's mockup: http://cl.ly/image/131K2d3U280f

In terms of the color of dimmed background, Shorlander should be the best person to make the call.
Flags: needinfo?(ywang)
(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #7)
Thanks Yuan!

> Rodrigo, we should add some padding on the left hand side of the
> auto-complete search results. The search results and title should be aligned
> at the same level with the URL field. See shorlander's mockup:
> http://cl.ly/image/131K2d3U280f
> 
Bug 904417 should take care of this.

> In terms of the color of dimmed background, Shorlander should be the best
> person to make the call.

OK, need info'd him.
Flags: needinfo?(shorlander)
Comment on attachment 803192 [details] [diff] [review]
898106.patch

Review of attachment 803192 [details] [diff] [review]:
-----------------------------------------------------------------

r=mbrubeck with some tiny nits

::: browser/metro/base/content/browser.xul
@@ +198,5 @@
>  
>          <!-- Content touch selection overlay -->
>          <box class="selection-overlay-hidden" id="content-selection-overlay"/>
> +
> +        <!-- Overlay to gray out start screen when autocomplete shows up -->

This comment should be updated (s/start screen/content/).

@@ +199,5 @@
>          <!-- Content touch selection overlay -->
>          <box class="selection-overlay-hidden" id="content-selection-overlay"/>
> +
> +        <!-- Overlay to gray out start screen when autocomplete shows up -->
> +        <hbox id="autocomplete-start-overlay" onclick="BrowserUI.blurNavBar()"/>

...and maybe this ID too.

::: browser/metro/theme/browser.css
@@ +231,5 @@
> +  right: 0;
> +  bottom: 0;
> +}
> +
> +#stack[autocomplete] #autocomplete-start-overlay {

This could be a child selector.
Attachment #803192 - Flags: review?(mbrubeck) → review+
Changing title to reflect new approach
Summary: Work - Make the autocomplete popup fully cover/replace the start page → Work - Dim screen when autocomplete popup shows up.
Attached patch 898106.patchSplinter Review
Clicking on content changed the behavior from dismissing autocomplete and navbar to dismissing only autocomplete. This caused a urlbar test to fail (yay test coverage!). Sorry I didn't catch this earlier, forgot to run tests after I made the change to dim on content.

Applied comments and some minor changes to the dismiss logic.
Attachment #803192 - Attachment is obsolete: true
Attachment #804016 - Flags: review?(mbrubeck)
Attachment #804016 - Flags: review?(mbrubeck) → review+
https://hg.mozilla.org/mozilla-central/rev/4160a3b1cca4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Clearing shorlander's needinfo since this has been in m-c for a while.
Flags: needinfo?(shorlander)
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.