1314983 - Synced Devices folder is not displayed in landscape when there is no browsing history

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: Awesomescreen, defect, P5)

Description

[Sorina Florean [:sflorean]]

Environment: 
Device: Nexus 5 (Android 6.0.1);
Build: Nightly 52.0a1 (2016-11-02);

Prerequisite:
1. Make sure Recently closed folder is displayed in History panel (there is at least one tab closed).
1. No browsing history is displayed.

Steps to reproduce:
1. Go to History Panel;
2. Turn the device in landscape mode;
3. Observe the Recent Closed and Synced Devices folders (scroll).

Expected result:
Synced Devices folder is displayed in landscape mode when there is no browsing history.

Actual result:
- Synced Devices folder is not displayed in landscape mode when there is no browsing history.
- Synced Devices folder is under scroll menu.

Notes:
Please check the video: https://youtu.be/LoUvT0e7qw0

Comment 1

[Jan Henning [:JanH]]

Compare also https://hg.mozilla.org/mozilla-central/rev/35e71e69db49.

We have since updated the support library, enabling the use of "wrap_content" for recycler views, although I didn't have any immediate success when giving this a try. Maybe somebody with more Android layout experience has better luck...

Comment 2

[Julian Chu [:walkingice]]

Attachment: Bug 1314983 - Increase height of history list

Some items become invisible due to the height of list is not enough in
landscape mode. This adjustment tries to ease the problem.

Review commit: https://reviewboard.mozilla.org/r/101312/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/101312/

Comment 3

[Julian Chu [:walkingice]]

Attachment: demostrace for 2 modes

The 'Synced Devices folder' exists but not visible in landscape mode. Send a patch to ease the problem by giving more height.

Comment 4

[Julian Chu [:walkingice]]

In comment 1, "wrap_content" doesn't work because the RecyclerView is wrapped in a SwipeRefreshLayout, and its height is measured by `layout_weight`.

Here comes a new UX question: The empty-view should be placed in the List, or out of the List?

Height of List is usually dynamic, and is scrollable.

Now the empty-view is out of List so we have this bug. Increase height just work for now (until we add one more item into List).If we move empty-view into List, then another UX issue occurs: the empty-view might be hidden until user scroll the list.

Anthony, do you have any idea to solve this situation?

Comment 5

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

I think this was something Liuche and I didn't have a chance to get to when we put in the smart folders in the History panel. 

(In reply to Julian Chu [:walkingice] from comment #4)
> Here comes a new UX question: The empty-view should be placed in the List,
> or out of the List?

Sorry, I'm not sure what you mean here. But from a UX point of view, the user should not be scrolling where Sorina is in her attached video. The folders should behave like they do in portrait mode.

> Height of List is usually dynamic, and is scrollable.
> 
> Now the empty-view is out of List so we have this bug. Increase height just
> work for now (until we add one more item into List).If we move empty-view
> into List, then another UX issue occurs: the empty-view might be hidden
> until user scroll the list.
>
> Anthony, do you have any idea to solve this situation?

I'm not concerned with the empty state UI being "off screen" in horizontal mode. But the smart folders should not be cropped like that. Even if it pushes the empty state off screen, they should not be scrollable like that.

Chenxia do you have more context? I remember you explained this to me once...

Comment 6

[Julian Chu [:walkingice]]

> (In reply to Julian Chu [:walkingice] from comment #4)
> > Here comes a new UX question: The empty-view should be placed in the List,
> > or out of the List?
> 
> Sorry, I'm not sure what you mean here. But from a UX point of view, the
> user should not be scrolling where Sorina is in her attached video. The
> folders should behave like they do in portrait mode.

Sorry I didn't explain it well. Let me try to explain it via attachment #8822374 [details]. Currently on screen there are two views: List and Empty-View. List occupy 28%(approximation) height, and Empty-View occupy 72%. If we move Empty-view into List, then List can occupy 100% height. But Empty-View might off screen, if there are many items in List.

Comment 7

[Julian Chu [:walkingice]]

Attachment: emptyview_in_list.png

If we put Empty-View into List, this might happen

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Julian Chu [:walkingice] from comment #7)
> Created attachment 8822590 [details]
> emptyview_in_list.png
> 
> If we put Empty-View into List, this might happen

I think this actually can't happen because we would not show the empty view in this case, right? But yeah on very small devices you might need to scroll a bit to see the empty view if the two folders take too much space and I think this is totally fine. We can't have both.

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8822368 [details]
Bug 1314983 - Increase height of history list

https://reviewboard.mozilla.org/r/101312/#review102474

So, I guess this is not the correct approach and instead we want to move the view inside the recyclerview, right?

Comment 10

[Julian Chu [:walkingice]]

(In reply to Sebastian Kaspari (:sebastian) from comment #9)
> Comment on attachment 8822368 [details]
> Bug 1314983 - Increase height of history list
> 
> https://reviewboard.mozilla.org/r/101312/#review102474
> 
> So, I guess this is not the correct approach and instead we want to move the
> view inside the recyclerview, right?

If we decide to move EmptyView into RecyclerView. We can do it in this bug, or file a new bug to do it. Both of two are OK to me!

Comment 11

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

(In reply to Julian Chu [:walkingice] from comment #10)
> (In reply to Sebastian Kaspari (:sebastian) from comment #9)
> > Comment on attachment 8822368 [details]
> > Bug 1314983 - Increase height of history list
> > 
> > https://reviewboard.mozilla.org/r/101312/#review102474
> > 
> > So, I guess this is not the correct approach and instead we want to move the
> > view inside the recyclerview, right?
> 
> If we decide to move EmptyView into RecyclerView. We can do it in this bug,
> or file a new bug to do it. Both of two are OK to me!

Are we talking about merging these two views into one?
Flagging Jack for overseeing UX impact.

Comment 12

[Julien Cristau [:jcristau]]

Mass wontfix for bugs affecting firefox 52.

Comment 13

[Firefox Bug Husbandry Bot]

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.

Comment 14

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.