This repository has been archived by the owner on Apr 17, 2021. It is now read-only.

Refactor overlay into split #1664

Closed

athomasmoz opened this issue Jan 11, 2019 · 3 comments

Assignees

Labels

size S

Milestone

athomasmoz commented Jan 11, 2019

Refactor overlay into split

This will involve adjusting fragment transactions so that WebRenderFragment is visible

Why/User Benefit/User Problem

What / Requirements

Acceptance Criteria (how do I know when I’m done?)

The text was updated successfully, but these errors were encountered:

athomasmoz added the size S label

athomasmoz added this to the 2019 Q1 SP2 milestone

athomasmoz mentioned this issue

[1304] Slide Up Overlay #1310

Closed

severinrudie mentioned this issue

Animate Scrolling as Focus Moves on Overlay #1669

Closed

athomasmoz mentioned this issue

Animate overlay entrance/removal #1666

Closed

athomasmoz modified the milestones: 2019 Q1 SP2, 2019 Q1 Feature Backlog, 2019 Q1 SP3

mcomella self-assigned this

Contributor

mcomella commented Jan 29, 2019

Presumably, the home page will stay the same but we'll want something like this while browsing:

We should design for:

Animate Scrolling as Focus Moves on Overlay #1669 Being able to animate the bottom layer over the top when focus changes or scrolling the whole page on the home screen
Change of internal layouts of each view: these will change soon so we want to make sure they're easy to change quickly

mcomella added a commit that referenced this issue


          Issue #1664: Move ToolbarViewModel to navigationoverlay pkg.

e0b3d58

The toolbar and navigationoverlay packages both had one item in them.
The toolbar is only used in the navigationoverlay so I didn't see any
reason to keep around two very small packages.

mcomella added a commit that referenced this issue


          Issue #1664: Move toolbar UI code to ToolbarUiController.

69e5a0f

The NavigationOverlayFragment is starting to get unwieldly: it's 550+
lines and it's hard to figure out which code is related to the toolbar
and which code is related to the bottom navigation panel. Since they're
being split, now is an opportune time to separate their file contents.
Moving the toolbar code out:
- Reduces NavOverlayFrag to ~430 lines
- Makes the dependencies between the toolbar and bottom navigation panel
more obvious
- Makes the dependencies between the toolbar and the framework more
obvious
- Makes the shared code more obvious

No functionality was changed and no code was cleaned up: I essentially
just moved functions, updated references, and added the necessary
parameters.

This refactor isn't perfect - e.g. should the NavOverlayFragment really
always handle `onNavigationEvent`? - but is intended to make things easier
to change later.

mcomella added a commit that referenced this issue


          Issue #1664: Make things in NavOverlayFragment private that can be.

0ab36e6

mcomella added a commit that referenced this issue


          Issue #1664: Remove unnecessary updateFocusableViews param.

5c3658f

The current focus is the default argument so it's unnecessary for us to
pass it in.

This simplifies the depependencies that ToolbarUiController has on the
NavOverlayFragment.

ghost added the in progress label

mcomella added a commit that referenced this issue


          Issue #1664: Move ToolbarViewModel to navigationoverlay pkg.

702c409

The toolbar and navigationoverlay packages both had one item in them.
The toolbar is only used in the navigationoverlay so I didn't see any
reason to keep around two very small packages.

mcomella added a commit that referenced this issue


          Issue #1664: Move toolbar UI code to ToolbarUiController.

1ac01f1

The NavigationOverlayFragment is starting to get unwieldly: it's 550+
lines and it's hard to figure out which code is related to the toolbar
and which code is related to the bottom navigation panel. Since they're
being split, now is an opportune time to separate their file contents.
Moving the toolbar code out:
- Reduces NavOverlayFrag to ~430 lines
- Makes the dependencies between the toolbar and bottom navigation panel
more obvious
- Makes the dependencies between the toolbar and the framework more
obvious
- Makes the shared code more obvious

No functionality was changed and no code was cleaned up: I essentially
just moved functions, updated references, and added the necessary
parameters.

This refactor isn't perfect - e.g. should the NavOverlayFragment really
always handle `onNavigationEvent`? - but is intended to make things easier
to change later.

mcomella added a commit that referenced this issue


          Issue #1664: Make things in NavOverlayFragment private that can be.

9f3821d

mcomella added a commit that referenced this issue


          Issue #1664: Remove unnecessary updateFocusableViews param.

ac3896a

The current focus is the default argument so it's unnecessary for us to
pass it in.

This simplifies the depependencies that ToolbarUiController has on the
NavOverlayFragment.

mcomella added a commit that referenced this issue


          Issue #1664 - post: Fix lint error on unused import.

fd4ef1e

mcomella added a commit that referenced this issue


          Issue #1664: Move ToolbarViewModel to navigationoverlay pkg.

0daf1ca

The toolbar and navigationoverlay packages both had one item in them.
The toolbar is only used in the navigationoverlay so I didn't see any
reason to keep around two very small packages.

mcomella added a commit that referenced this issue


          Issue #1664: Move toolbar UI code to ToolbarUiController.

9126e3b

The NavigationOverlayFragment is starting to get unwieldly: it's 550+
lines and it's hard to figure out which code is related to the toolbar
and which code is related to the bottom navigation panel. Since they're
being split, now is an opportune time to separate their file contents.
Moving the toolbar code out:
- Reduces NavOverlayFrag to ~430 lines
- Makes the dependencies between the toolbar and bottom navigation panel
more obvious
- Makes the dependencies between the toolbar and the framework more
obvious
- Makes the shared code more obvious

No functionality was changed and no code was cleaned up: I essentially
just moved functions, updated references, and added the necessary
parameters.

This refactor isn't perfect - e.g. should the NavOverlayFragment really
always handle `onNavigationEvent`? - but is intended to make things easier
to change later.

mcomella added a commit that referenced this issue


          Issue #1664: Make things in NavOverlayFragment private that can be.

d252271

mcomella added a commit that referenced this issue


          Issue #1664: Remove unnecessary updateFocusableViews param.

1ba89eb

The current focus is the default argument so it's unnecessary for us to
pass it in.

This simplifies the depependencies that ToolbarUiController has on the
NavOverlayFragment.

mcomella added a commit that referenced this issue


          Issue #1664 - post: Fix lint error on unused import.

62f3880

mcomella added a commit that referenced this issue


          Issue #1664: Move ToolbarViewModel to navigationoverlay pkg.

a18dc5d

The toolbar and navigationoverlay packages both had one item in them.
The toolbar is only used in the navigationoverlay so I didn't see any
reason to keep around two very small packages.

mcomella added a commit that referenced this issue


          Issue #1664: Move toolbar UI code to ToolbarUiController.

54d5362

The NavigationOverlayFragment is starting to get unwieldly: it's 550+
lines and it's hard to figure out which code is related to the toolbar
and which code is related to the bottom navigation panel. Since they're
being split, now is an opportune time to separate their file contents.
Moving the toolbar code out:
- Reduces NavOverlayFrag to ~430 lines
- Makes the dependencies between the toolbar and bottom navigation panel
more obvious
- Makes the dependencies between the toolbar and the framework more
obvious
- Makes the shared code more obvious

No functionality was changed and no code was cleaned up: I essentially
just moved functions, updated references, and added the necessary
parameters.

This refactor isn't perfect - e.g. should the NavOverlayFragment really
always handle `onNavigationEvent`? - but is intended to make things easier
to change later.

mcomella added a commit that referenced this issue


          Issue #1664: Make things in NavOverlayFragment private that can be.

fa5b117

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Move nav bar out of scrollview

c202a8e

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Switch to CoordinatorLayout and move Brow…

4afa4f6

…ser*ScrollView

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Fix toolbar padding.

9c76d47

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Add navigation overlay toolbar-push behavior

86985a4

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Move toolbar below sliding overlay

f3dd35e

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Different split behavior for homescreen a…

1c1a4a4

…nd webpage

Contributor

liuche commented Feb 23, 2019

I'm also sending @brampitoyo a build so I can get some feedback on various sizing of the toolbar, window space, etc, but since this is landing turned off I can file the polish in a later bug.

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Rename browser_overlay to navigation_overlay

081bc06

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Make copy of navigation overlay layout fo…

44647a1

…r A/B testing

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Move nav bar out of scrollview

c790182

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Switch to CoordinatorLayout and move Brow…

555e9fc

…ser*ScrollView

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Fix toolbar padding.

13ae8df

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Add navigation overlay toolbar-push behavior

de99648

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Move toolbar below sliding overlay

92d9690

liuche added a commit to liuche/firefox-tv that referenced this issue


          Issue mozilla-mobile#1664 - Different split behavior for homescreen a…

81f672d

…nd webpage

liuche mentioned this issue

Follow-up: Add the tests for split overlay #1878

Closed

liuche added a commit that referenced this issue


          Issue #1664 - Rename browser_overlay to navigation_overlay

57f1ee7

liuche added a commit that referenced this issue


          Issue #1664 - Make copy of navigation overlay layout for A/B testing

ca2ceb7

liuche added a commit that referenced this issue


          Issue #1664 - Move nav bar out of scrollview

25413f1

liuche added a commit that referenced this issue


          Issue #1664 - Switch to CoordinatorLayout and move Browser*ScrollView

555a172

liuche added a commit that referenced this issue


          Issue #1664 - Fix toolbar padding.

048b71b

liuche added a commit that referenced this issue


          Issue #1664 - Add navigation overlay toolbar-push behavior

abf3b1b

liuche added a commit that referenced this issue


          Issue #1664 - Move toolbar below sliding overlay

9cb2bee

liuche added a commit that referenced this issue


          Issue #1664 - Different split behavior for homescreen and webpage

53b648a

liuche closed this as completed

ghost removed the in progress label

This was referenced Feb 25, 2019

[1304] One-line Browser Toolbar #1315

Closed

Follow-up: Add animation for split overlay - "push toolbar" #1884

Closed

Regression: Fragment changes do not handle focus correctly #1888

Closed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.