Skip to content
This repository was archived by the owner on Oct 1, 2018. It is now read-only.

Conversation

@feloy
Copy link
Contributor

@feloy feloy commented Oct 2, 2017

Here a proposition for the sidenav design

@ladyleet ladyleet merged commit 19b340e into ReactiveX:master Oct 2, 2017
@ladyleet
Copy link
Member

ladyleet commented Oct 2, 2017

@feloy this is really helpful thank you so much for doing that! 👍 merged.

@@ -1,10 +1,37 @@
import { Component } from '@angular/core';

class Menu {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we update this to an interface?

Copy link
Member

Choose a reason for hiding this comment

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

@feloy thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll do that

@@ -1,37 +1,24 @@
<md-sidenav-container fullscreen>
<div class="app-fullpage">
<md-toolbar color="accent" class="mat-elevation-z6">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should make the toolbar sticky, thoughts? @ladyleet @feloy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll look how to do this. Previous solution wasn't perfect because sidenav was on top of toolbar.
It seems that "md-sidenav-container fullscreen" is not usable with a toolbar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

capture du 2017-10-03 19 43 28

In fact the toolbar is already sticky, isn't it? At least on Chrome and Firefox on Linux

Copy link
Collaborator

Choose a reason for hiding this comment

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

Apologies, just tested again and it looks great! 👍

@ladyleet
Copy link
Member

ladyleet commented Oct 3, 2017

@btroncone I don't see why not! @feloy?

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.

3 participants