Skip to content
This repository has been archived by the owner on Jan 19, 2023. It is now read-only.

Create a bottom panel to show secondary content #1341

Merged
merged 3 commits into from
Sep 17, 2020
Merged

Conversation

bryanl
Copy link
Contributor

@bryanl bryanl commented Sep 13, 2020

What this PR does / why we need it:

This set of changes creates a bottom panel component to show secondary content. This change is meant to supersede the functionality provided in the SliderView component by introducing the following features:

  • Only includes a bottom panel and not the contents of the bottom panel. This reduces coupling and allows the parent component to decide what is shown.
  • Allows for resizing the content without using the "ghost" method that was previously employed
  • Is designed to live in the same container as other content rather than as an overlay. ie. It will push up content when it is activated.

This change also includes:

  • Temporarily makes the collapsed navigation view scrollable.

Special notes for your reviewer:

The panel is included, but it is not activated. To see it working, set isBottomPanelEnabled to true in ContainerComponent and add some content to app-bottom-panel in the view.

@bryanl bryanl requested a review from a team September 13, 2020 13:05
@bryanl bryanl self-assigned this Sep 13, 2020
@bryanl bryanl marked this pull request as draft September 15, 2020 11:59
@mklanjsek
Copy link
Contributor

I like this, except one thing: IMO collapsed navigation should never be scrollable. Can the slider be positioned inside content-area so the navigation still consumes the whole height of the window?

@bryanl bryanl force-pushed the create-bottom-panel branch 2 times, most recently from a934972 to 907d40d Compare September 17, 2020 12:42
@bryanl bryanl marked this pull request as ready for review September 17, 2020 12:44
The bottom panel positions itself at the bottom of the container. It can be activated and resized to show secondary content.

Signed-off-by: bryanl <bryanliles@gmail.com>
The bottom panel is disabled by default until it is fully implemented

Signed-off-by: bryanl <bryanliles@gmail.com>
This is a temporary fix to allow the collapsed navigation to scroll. The ultimate fix would require navigation to present an overflow menu if it can display its full height.

Signed-off-by: bryanl <bryanliles@gmail.com>
Copy link
Contributor

@mklanjsek mklanjsek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this, but I am seeing following problems when testing it:

  • after navigating from one page to another, the bottom panel collapses to its minimized state - I would expect it to stay open,
  • the compressed side nav is broken after scrollbar has been added - it's impossible to select any items in flyout menu. The flyout menu shows up briefly, but then disappears after pointer is moved over the scrollbar,
  • As commented above, I still think that bottom panel has to be be positioned inside the main pane and left navigation should go all the way to the bottom of the window,

@bryanl
Copy link
Contributor Author

bryanl commented Sep 17, 2020

@mklanjsek

  • Yes, the panel will collapse. It's not fully integrated as of this commit, so it is disabled. I have code that'll fix that in a future commit.
  • I believe this bug is in the compressed size nav code. The menu isn't updating itself properly, and this will need to be fixed before we enable this new feature.
  • I'll move the panel inside, but as noted in Left Navigation redesign #1353, we'll need to a better job with the sidebar menu to make it more usable. What I'm going to do is remove the bottom panel component from the container component for now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants