Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: sidebar - implement component/module #534

Closed
catlabs opened this issue Jul 19, 2018 · 4 comments
Closed

ui: sidebar - implement component/module #534

catlabs opened this issue Jul 19, 2018 · 4 comments

Comments

@catlabs
Copy link
Contributor

catlabs commented Jul 19, 2018

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/NationalBankBelgium/stark/blob/master/CONTRIBUTING.md#got-a-question-or-problem

Expected behavior

Separate module including component(s), styling and all the API related to this feature.
Sidebar component and possibility to have one left and one right sidebar.

What is the motivation / use case for changing the behavior?

See #104

@catlabs
Copy link
Contributor Author

catlabs commented Jul 19, 2018

So we should decide what to do with the sidebar, from what I have seen, we could make the Angular Material sidebar with the old API with a minor change on the app structure

Old Stark:

<main-content></main-content>
<stark-app-sidebar position="left">
  <stark-sidebar-top-slot>This is a LEFT side panel (top)</stark-sidebar-top-slot>
  <stark-sidebar-middle-slot>You can provide your content here (middle)</stark-sidebar-middle-slot>
  <stark-sidebar-bottom-slot>And here as well (bottom)</stark-sidebar-bottom-slot>
</stark-app-sidebar>

<stark-app-sidebar position="right">
  <stark-sidebar-top-slot>This is a RIGHT side panel (top)</stark-sidebar-top-slot>
  <stark-sidebar-middle-slot>You can provide your content here (middle)</stark-sidebar-middle-slot>
  <stark-sidebar-bottom-slot>And here as well (bottom)</stark-sidebar-bottom-slot>
</stark-app-sidebar>

Angular Material:

<mat-sidenav-container>
  <mat-sidenav opened mode="side">Start content</mat-sidenav>
  <mat-sidenav opened mode="side" position="end">End content</mat-sidenav>
  <mat-sidenav-content>
    <main-content></main-content>
  </mat-sidenav-content>
</mat-sidenav-container>

As you can see, the main-content has to be placed now inside the sidenav-container so we have to change the API if we want to use Angular Material sidenav.
There is 3 options:

  • We keep the old one
  • We give up on stark-sidebar and recommand to use Angular Material
  • We create a wrapper around the mat-sidenav and work with transclusion. The code needs to be adapted and will look like this
<stark-app-sidebar-container>
  <stark-app-sidebar position="left">
    <stark-sidebar-top-slot>This is a LEFT side panel (top)</stark-sidebar-top-slot>
    <stark-sidebar-middle-slot>You can provide your content here (middle)</stark-sidebar-middle-slot>
    <stark-sidebar-bottom-slot>And here as well (bottom)</stark-sidebar-bottom-slot>
  </stark-app-sidebar>

  <stark-app-sidebar position="right">
    <stark-sidebar-top-slot>This is a RIGHT side panel (top)</stark-sidebar-top-slot>
    <stark-sidebar-middle-slot>You can provide your content here (middle)</stark-sidebar-middle-slot>
    <stark-sidebar-bottom-slot>And here as well (bottom)</stark-sidebar-bottom-slot>
  </stark-app-sidebar>
  <stark-app-sidebar-content></stark-app-sidebar-content>
</stark-app-sidebar-container>

I think the third option is the best cause we will more or less keep the same API and also benefit from the features of the Angular Material sidenav.

@christophercr @SuperITMan what do you think?

@christophercr
Copy link
Collaborator

Indeed, I remember that with Angular Material 1 there was a similar restriction: to put the content and the sidenav as siblings inside the same container. So now in Angular Material 2 is more explicit :p

I think that option 3 is the best since we need to keep the same API (which is minimal in the stark-sidebar) and also to provide a service to interact with the sidebar.

I am wondering though, what will we do about the app-menu component? That component is provided in the old Stark and it is similar to the sidebar in the sense that it shows a panel to the left or right but it is totally different regarding its content.

In the old Stark, an app could have a sidenav (app-menu component) and 2 sidebars (app-sidebar component left and right). With the new Angular Material 2 API and the fact that they require the <mat-sidenav-container>, would this be feasible? Or maybe we should re-think whether it makes sense to have 2 separate components...

@catlabs
Copy link
Contributor Author

catlabs commented Jul 19, 2018

About the app-menu, I think it doesn't make sense to have the same kind of logic than in the app-sidebar, they should just be combined. Best practice (dumb components) would be to have a app-menu that only takes care of displaying the menu/submenus, so we could also use it somewhere else in the app if needed.

About the 2 left sidebar, well yes we could do something that is close to the old one by displaying/adding 2 sidenav-content on the left sidenav. But does it really make sense to have 3 sidebars in the app?

@SuperITMan SuperITMan added this to To do in 10.0.0-alpha.4 Jul 27, 2018
@christophercr christophercr added this to To do in 10.0.0-alpha.5 via automation Jul 30, 2018
@christophercr christophercr removed this from To do in 10.0.0-alpha.4 Jul 30, 2018
@christophercr
Copy link
Collaborator

Closed in favor of #592

10.0.0-alpha.5 automation moved this from To do to Done Sep 6, 2018
@christophercr christophercr removed this from the 10.0.0-alpha.5 milestone Sep 6, 2018
@christophercr christophercr removed this from Done in 10.0.0-alpha.5 Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants