801000 - Work - Implement Desired Snapped View

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: General, defect)

Comment 1

[Asa Dotzler [:asa]]

I'm pretty sure (at least the core feature of) this work is done. Firefox handles the snapped and filled views in landscape mode (expected) on my Samsung Tablet.

Comment 2

[Allison Naaktgeboren :ally]

After talking with yuan, the snapped view is going to look & behave very differently on the awesome screen, essentially be a totally different screen. 

Right now, it is just a shortened view of the awesomescreen that is not scrollable and not terribly awesome. 

Topsites may not be thumbnails here but favicons. (after 794028 lands)

yuan has some mocks that should be appearing soon.

Comment 3

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: Start page in snap and fill view

Start page in snap view has top sites listed in favicon + title, instead of in thumbnails.
Start page in fill view basically behaves the same with full-screen view. There needs to be a horizontal scrolling bar in fill view.

Note: This is an old mockup. "Pinned Sites" should be replaced by "Synced Tabs"

Comment 4

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: Awesomescreen search in snap and full-screen view

Awesomescreen search should be supported in snap view for users to explore to other sites. Based on the mockup above, awesomescreen shows your up to 5 search results. "Internet Search" options are always down at the bottom. 

Since users could be typing with soft keyboard open, which will cover almost half of the space on the snapped screen. So scrolling bar should be avoided on snapped awesomescreen and start page. 


Note: Tab switching, app bar will not be available in snap view. They don't fit into the user scenarios of using snap view.

Comment 5

[Jim Mathies [:jimm]]

Ally - plz update if this loe is off.

Comment 6

[Allison Naaktgeboren :ally]

Attachment: wip

has:
- toggles between states snapped/all others
- has functional snappedview screen
- bookmarks, remote tabs link to containers
- entries for all links on mock (history & pinned sites n/a see below)

does not have:
- topsites 9x1 grid
- url bar
- remote tabs
- prettiness
- tests

additional work/followup (on mocks but these features don't exist)
- pinned sites container
- recent history container

Comment 7

[Allison Naaktgeboren :ally]

Attachment: wip 1

yay workweek. refactor of history and topsites so they can be used in snapped view.

Comment 8

[Allison Naaktgeboren :ally]

Attachment: splitting out refactor

Comment 9

[Matt Brubeck (:mbrubeck)]

Comment on attachment 703079 [details] [diff] [review]
splitting out refactor

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

::: browser/metro/base/content/TopSites.js
@@ +5,5 @@
> +
> +'use strict';
> + let prefs = Components.classes["@mozilla.org/preferences-service;1"].
> +      getService(Components.interfaces.nsIPrefBranch);
> +//debug -

left-over comment?

Comment 10

[Allison Naaktgeboren :ally]

refactoring doesn't actually have a bug, so I guess I just broke process there. :/

https://hg.mozilla.org/projects/elm/rev/e812b21023a8

Comment 11

[Allison Naaktgeboren :ally]

Attachment: wip 2 -snapped view as seperate page in <stack>

post refactor & going to explore a different design based on new requirements from yesterday's meeting

Comment 12

[Allison Naaktgeboren :ally]

Attachment: wip 0 snappedview2, child of <id=page>

Comment 13

[Allison Naaktgeboren :ally]

Attachment: wip 1 snappedview2, child of <id=page>

missing:
- correct handling should we start in snapped view
- autocomplete screen not coming up in snapped view

- I would like to clean up StartUI object & methods once everything is straightened out

Comment 15

[Matt Brubeck (:mbrubeck)]

Comment on attachment 704723 [details] [diff] [review]
wip 1 snappedview2, child of <id=page>

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

::: browser/metro/base/content/browser.js
@@ +1935,5 @@
>      os.addObserver(this, "metro_softkeyboard_hidden", false);
> +    os.addObserver(this, "metro_viewstate_snapped", false);
> +    os.addObserver(this, "metro_viewstate_landscape", false);
> +    os.addObserver(this, "metro_viewstate_filled", false);
> +    os.addObserver(this, "metro_viewstate_portrait", false);

Instead of listening for these here, could we have these observers replace the existing "SizeChanged" listener in BrowserUI instead, and combine the logic from this patch with that logic?  Here's a sketch to demonstrate what I mean; it also seems to fix some bugs with toolbar styles not changing at the right times:

http://people.mozilla.com/~mbrubeck/prototype.diff

Comment 16

[Allison Naaktgeboren :ally]

Attachment: draft 2 part 2/3, css

I concede that mbrubeck & fryn were right. the css route is less tangled in this case.

- This patch includes everything except the colored topsites 9x1 grid (to become part 3/3; part 1/3 was the refactor above)

- A couple of the selectors can be grouped together, if you like. I had them broken apart for testing & my own clarity. 

- I understand the descendant selector is expensive & ordinarily forbidden in firefox ux css. I have tried to replace them as best I can, but I am a css noob, perhaps there is a cheaper/cleaner way?

Comment 17

[Matt Brubeck (:mbrubeck)]

Comment on attachment 706667 [details] [diff] [review]
draft 2 part 2/3, css

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

Ship it!

::: browser/metro/theme/browser.css
@@ +625,5 @@
>  #start-container {
>    margin-top: -@metro_border_thin@;
>  }
>  
> +#start-scrollbox {

Thanks for cleaning the inefficient CSS here!

@@ +650,2 @@
>    visibility: collapse;
>  }

(In reply to :Ally Naaktgeboren from comment #16)
> - I understand the descendant selector is expensive & ordinarily forbidden
> in firefox ux css. I have tried to replace them as best I can, but I am a
> css noob, perhaps there is a cheaper/cleaner way?

A slightly cleaner option might be to set the "snapped" attribute on a <broadcaster> element instead of on the #stack, and then make #start-container or its children "observe" that broadcaster.  For some example code, look at "bcast_urlbarState" here, and the various elements with observes="bcast_urlbarState" elsewhere:
http://hg.mozilla.org/projects/elm/file/f78d718ea05f/browser/metro/base/content/browser.xul#l47

and here are some of the CSS rules that use the "mode" attribute set through that broadcaster:
http://hg.mozilla.org/projects/elm/file/f78d718ea05f/browser/metro/theme/browser.css#l450

You can make this change in this patch (and carry the r+), or it's also fine to land what you have here and we can decide about the broadcaster in a followup.

Comment 18

[Allison Naaktgeboren :ally]

https://hg.mozilla.org/projects/elm/rev/5d48d1fadc7d