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

Allow paper-menu's to be displayed with md-dense. #303

Merged
merged 2 commits into from
Feb 28, 2016

Conversation

DanChadwick
Copy link
Contributor

Angular defines a md-dense style, but because it needs to be applied in the paper-menu-content-pane, which is in the paper-menu-content, which is in turn in the paper-menu, there is no easy way for md-dense to be specified. This PR adds a "dense=true" option to paper-menu, which "bubbles down" (sinks??) down to the paper-menu-content-pane, where it adds the required class.

@DanChadwick
Copy link
Contributor Author

Again, test failure is unrelated to this PR.

@miguelcobain
Copy link
Owner

The md-dense class is only applied in the menu bar directive: https://material.angularjs.org/latest/demo/menuBar

I'm divided. On one side, we should encourage following the material design spec. And it looks like the material design spec doesn't have "dense" menus outside menu bars? (need confirmation)
On the other hand, this would make the component slightly more flexible, at little cost (the styles are there). Also, I'm afraid that AM suddenly ceases to support that outside menu bars and we're left with a burden because we've provided it to people in the first place.

I think this could be added, as it will help us when we develop menu-bars.

@miguelcobain
Copy link
Owner

Ah, looks like material design mentions nothing about dense lists only on menu bars: https://www.google.com/design/spec/components/lists.html#lists-specs

@DanChadwick can you please add an entry to CHANGELOG.md pointing to this issue?
I'm trying to not lose track of what's added in v1.0.0. People will need that later.

@DanChadwick
Copy link
Contributor Author

Changelog updated, and again the failure is unrelated to this PR.

miguelcobain added a commit that referenced this pull request Feb 28, 2016
Allow paper-menu's to be displayed with md-dense.
@miguelcobain miguelcobain merged commit 3195315 into miguelcobain:wip/v1.0.0 Feb 28, 2016
@DanChadwick DanChadwick deleted the dense-menus branch February 29, 2016 01:52
@DanChadwick
Copy link
Contributor Author

Thanks again. And on a Sunday, no less.

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

Successfully merging this pull request may close these issues.

None yet

2 participants