Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

mdMenu - a menu component - wip PR #3173

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
232 changes: 232 additions & 0 deletions src/components/menu/_menu.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for the naming convention of _menu?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ensure that it gets concatenated first by the build, important because it defines the module

* @ngdoc module
* @name material.components.menu
*/

angular.module('material.components.menu', [
'material.core',
'material.components.backdrop'
])
.directive('mdMenu', MenuDirective);

/**
* @ngdoc directive
* @name mdMenu
* @module material.components.menu
* @restrict E
* @description
*
* Menus are elements that open when clicked. They are useful for displaying
* additional options within the context of an action.
*
* Every `md-menu` must specify exactly two child elements. The first element is what is
* left in the DOM and is used to open the menu. This element is called the origin element.
Copy link
Member

Choose a reason for hiding this comment

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

I would use the term trigger over origin

* The origin element's scope has access to `$mdOpenMenu()`
* which it may call to open the menu.
*
* The second element is the `md-menu-content` element which represents the
* contents of the menu when it is open. Typically this will contain `md-menu-item`s,
* but you can do custom content as well.
*
* <hljs lang="html">
* <md-menu>
* <!-- Origin element is a md-button with an icon -->
* <md-button ng-click="$mdOpenMenu()" class="md-icon-button">
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of making the trigger more explicit with its own element? E.g.,

  <md-menu>
    <md-menu-trigger> ... </md-menu-trigger>
    <md-menu-content> ... </md-menu-content>
  </md-menu>

Copy link
Member

Choose a reason for hiding this comment

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

The button needs an aria-label

* <md-icon md-svg-icon="call:phone"></md-icon>
* </md-button>
* <md-menu-content>
* <md-menu-item><md-button ng-click="doSomething()">Do Something</md-button></md-menu-item>
* </md-menu-content>
* </md-menu>
* </hljs>

* ## Sizing Menus
*
* The width of the menu when it is open may be specified by specifying a `width`
Copy link
Member

Choose a reason for hiding this comment

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

Why not just use CSS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We had multiple issues with select that were a result of people not writing their own CSS. Typically, I am all for it. That being said, this gives them an easy way to follow the spec (which changes on mobile vs larger) using attrs. Internally we implement it purely with css.

* attribute on the `md-menu-content` element.
* See the [Material Design Spec](http://www.google.com/design/spec/components/menus.html#menus-specs)
* for more information.
*
*
* ## Aligning Menus
*
* When a menu opens, it is important that the content aligns with the origin element.
* Failure to align menus can result in jarring experiences for users as content
* suddenly shifts. To help with this, `md-menu` provides serveral APIs to help
* with alignment.
*
* ### Target Mode
*
* By default, `md-menu` will attempt to align the `md-menu-content` by aligning
* designated child elements in both the origin and the menu content.
*
* To specify the alignment element in the `origin` you can use the `md-menu-origin`
* attribute on a child element. If no `md-menu-origin` is specified, the `md-menu`
* will be used as the origin element.
*
* Similarly, the `md-menu-content` may specify a `md-menu-align-target` for a
* `md-menu-item` to specify the node that it should try and allign with.
Copy link
Member

Choose a reason for hiding this comment

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

allign > align

*
* In this example code, we specify an icon to be our origin element, and an
* icon in our menu content to be our alignment target. This ensures that both
* icons are aligned when the menu opens.
*
* <hljs lang="html">
* <md-menu>
* <md-button ng-click="$mdOpenMenu()" class="md-icon-button">
* <md-icon md-menu-origin md-svg-icon="call:phone"></md-icon>
* </md-button>
* <md-menu-content>
* <md-menu-item>
* <md-button ng-click="doSomething()">
* <md-icon md-menu-align-target md-svg-icon="call:phone"></md-icon>
* Do Something
* </md-button>
* </md-menu-item>
* </md-menu-content>
* </md-menu>
* </hljs>
*
* Sometimes we want to specify alignment on the right side of an element, for example
* if we have a menu on the right side a toolbar, we want to right align our menu content.
*
* We can specify the origin by using the `md-position-mode` attribute on both
Copy link
Member

Choose a reason for hiding this comment

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

"specify the origin" is confusing if the element itself is called the origin element.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with switching origin element -> triggering element and origin to be the alignment element. Good call

* the `x` and `y` axis. Right now only the `x-axis` has more than one option.
Copy link
Member

Choose a reason for hiding this comment

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

This sentence is rather confusing.

* You may specify the default mode of `target target` or
* `target-right target` to specify a right-oriented alignment target. See the
* position section of the demos for more examples.
*
* ### Menu Offsets
*
* It is sometimes unavoidable to need to have a deeper level of control for
* the positioning of a menu to ensure perfect alignment. `md-menu` provides
* the `md-offset` attribute to allow pixel level specificty of adjusting the
* exact positioning.
*
* This offset is provided in the format of `x y` or `n` where `n` will be used
* in both the `x` and `y` axis.
*
* For example, to move a menu by `2px` from the top, we can use:
* <hljs lang="html">
* <md-menu md-offset="2 0">
* <!-- menu-content -->
* </md-menu>
* </hljs>
*
* @usage
* <hljs lang="html">
* <md-menu>
* <md-button ng-click="$mdOpenMenu()" class="md-icon-button">
* <md-icon md-svg-icon="call:phone"></md-icon>
* </md-button>
* <md-menu-content>
* <md-menu-item><md-button ng-click="doSomething()">Do Something</md-button></md-menu-item>
* </md-menu-content>
* </md-menu>
* </hljs>
*
* @param {string} md-position-mode The position mode in the form of
`x`, `y`. Default value is `target`,`target`. Right now the `x` axis
also suppports `target-right`.
* @param {string} md-offset An offset to apply to the dropdown after positioning
`x`, `y`. Default value is `0`,`0`.
*
*/

function MenuDirective($mdMenu) {
return {
restrict: 'E',
require: 'mdMenu',
controller: function() { }, // empty function to be built by link
Copy link
Contributor

Choose a reason for hiding this comment

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

Should you use angular.noop?

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 need a new fn because otherwise I'd pollute angular.noop with a bunch of properties in the link function below

Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason you wouldn't use an actual controller for that?

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 need access the the el. So either the link function can build the controller, or I can pass inject $element into the controller. The real point is to provide a nice controller for other services (such as the interimElement) or sibling menus to use.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can include $element in a controller, but there's no way to include attr to my knowledge - still, I typically give my controller an init function that I call from the link if I need to pass anything. Makes it easier to follow (a pattern I picked up from Jeremy with keeping only the bare minimum code in the link method).

Copy link
Member

Choose a reason for hiding this comment

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

In a directive's controller, you can inject:

function MyDirectiveController($scope, $element, $attrs, $transclude) { }

I agree that just having a defined controller is the way to go (and is much more in line w/ Angular 2)

scope: true,
compile: compile
};

function compile(tEl) {
Copy link
Member

Choose a reason for hiding this comment

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

tEl > templateElement

tEl.addClass('md-menu');
tEl.children().eq(0).attr('aria-haspopup', 'true');
return link;
}

function link(scope, el, attrs, mdMenuCtrl) {
Copy link
Member

Choose a reason for hiding this comment

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

el > element

// Se up mdMenuCtrl to keep our code squeaky clean
buildCtrl();

// Expose a open function to the child scope for their html to use
scope.$mdOpenMenu = function() {
mdMenuCtrl.open();
};

if (el.children().length != 2) {
throw new Error('Invalid HTML for md-menu. Expected two children elements.');
Copy link
Member

Choose a reason for hiding this comment

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

Fun fact: you can just do throw Error('bad'); (without the new)

}

// Move everything into a md-menu-container
var menuContainer = angular.element('<div class="md-open-menu-container md-whiteframe-z2"></div>');
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for not using a template?

var menuContents = el.children()[1];
menuContainer.append(menuContents);

var enabled;
mdMenuCtrl.enable();

function buildCtrl() {
mdMenuCtrl.enable = function enableMenu() {
if (!enabled) {
//el.on('keydown', handleKeypress);
enabled = true;
}
};

mdMenuCtrl.disable = function disableMenu() {
if (enabled) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need this if check here. Actually, I believe even addEventListener will no-op if the arguments are identical.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it's commented out anyway, mdMenuCtrl.enable and mdMenuCtrl.disable are useless code at the moment, so I just removed them entirely.

//el.off('keydown', handleKeypress);
enabled = false;
}
};

mdMenuCtrl.open = function openMenu() {
el.attr('aria-expanded', 'true');
$mdMenu.show({
mdMenuCtrl: mdMenuCtrl,
element: menuContainer,
target: el[0]
});
};

mdMenuCtrl.close = function closeMenu(skipFocus) {
el.attr('aria-expanded', 'false');
$mdMenu.hide();
if (!skipFocus) el.children()[0].focus();
Copy link
Member

Choose a reason for hiding this comment

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

Some single-statement if blocks use braces and some don't; it should be consistent.

My preference is for always using braces (and on a separate line)

};

mdMenuCtrl.positionMode = function() {
var attachment = (attrs.mdPositionMode || 'target').split(' ');

if (attachment.length == 1) { attachment.push(attachment[0]); }
Copy link
Member

Choose a reason for hiding this comment

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

It's not very clear what/why this line is doing


return {
left: attachment[0],
top: attachment[1]
};

};

mdMenuCtrl.offsets = function() {
Copy link
Member

Choose a reason for hiding this comment

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

I'd like all of these controller functions to have descriptions, but this one in particular. A method name of offsets does not immediately convey what it does.

var offsets = (attrs.mdOffset || '0 0').split(' ').map(function(x) { return parseFloat(x, 10); });
Copy link
Member

Choose a reason for hiding this comment

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

Can you try to keep all lines <= 100 cols?

if (offsets.length == 2) {
return {
left: offsets[0],
top: offsets[1]
};
} else if (offsets.length == 1) {
return {
top: offsets[0],
left: offsets[0]
};
} else {
throw new Error('Invalid offsets specified. Please follow format <x, y> or <n>');
}
};
}
}
}
33 changes: 33 additions & 0 deletions src/components/menu/demoBasicUsage/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<div class="md-menu-demo" ng-controller="BasicDemoCtrl as ctrl">

<div class="menu-demo-container" layout-align="center center" layout="column">
<h2 class="md-title">Simple dropdown menu</h2>
<p>Note that applying the <code>md-menu-origin</code> and <code>md-menu-align-target</code> attributes ensure that the menu elements align</p>
<md-menu>
<md-button aria-label="Open phone interactions menu" class="md-icon-button" ng-click="$mdOpenMenu()">
<md-icon md-menu-origin md-svg-icon="call:phone"></md-icon>
</md-button>
<md-menu-content width="4">
<md-menu-item>
<md-button ng-click="ctrl.redial($event)">
<md-icon md-svg-icon="call:dialpad" md-menu-align-target></md-icon>
Redial
</md-button>
</md-menu-item>
<md-menu-item>
<md-button disabled="disabled" ng-click="ctrl.checkVoicemail()">
<md-icon md-svg-icon="call:voicemail"></md-icon>
Check voicemail
</md-button>
</md-menu-item>
<md-menu-divider></md-menu-divider>
<md-menu-item>
<md-button ng-click="ctrl.toggleNotifications()">
<md-icon md-svg-icon="social:notifications-{{ctrl.notificationsEnabled ? 'off' : 'on'}}"></md-icon>
{{ctrl.notificationsEnabled ? 'Disable' : 'Enable' }} notifications
</md-button>
</md-menu-item>
</md-menu-content>
</md-menu>
</div>
</div>
28 changes: 28 additions & 0 deletions src/components/menu/demoBasicUsage/script.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
angular.module('menuDemoBasic', ['ngMaterial'])
.config(function($mdIconProvider) {
$mdIconProvider
.iconSet("call", '/img/icons/sets/communication-icons.svg', 24)
.iconSet("social", '/img/icons/sets/social-icons.svg', 24);
})
.controller('BasicDemoCtrl', DemoCtrl);

function DemoCtrl($mdDialog) {
var vm = this;
vm.notificationsEnabled = true;
vm.toggleNotifications = function() {
vm.notificationsEnabled = !vm.notificationsEnabled;
};

vm.redial = function(e) {
$mdDialog.show(
$mdDialog.alert()
.title('Suddenly, a redial')
.content('You just called someone back. They told you the most amazing story that has ever been told. Have a cookie.')
.ok('That was easy')
);
};

vm.checkVoicemail = function() {
// This never happens.
};
}
7 changes: 7 additions & 0 deletions src/components/menu/demoBasicUsage/style.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.md-menu-demo {
padding: 24px;
}

.menu-demo-container {
min-height: 200px;
}
56 changes: 56 additions & 0 deletions src/components/menu/demoMenuPositionModes/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<div class="md-menu-demo" ng-controller="PositionDemoCtrl as ctrl">
<div class="menu-demo-container" layout-align="center center" layout="column">
<h2 class="md-title">Positon Mode Demos</h2>
<p>The <code>md-position-mode</code> attribute can be used to specify the positioning along the <code>x</code> and <code>y</code> axis.</p>
<h3 class="md-subhead">Target-Based Position Modes</h3>
<div class="menus" layout-fill layout-wrap layout="row" layout-align="space-between center">
<div layout="column" flex="33" flex-sm="100" layout-align="center center">
<p>Target Mode Positioning (default)</p>
<md-menu>
<md-button aria-label="Open demo menu" class="md-icon-button" ng-click="$mdOpenMenu()">
<md-icon md-menu-origin md-svg-icon="call:business"></md-icon>
</md-button>
<md-menu-content width="6">
<md-menu-item ng-repeat="item in [1, 2, 3]">
<md-button ng-click="ctrl.announceClick($index)">
<md-icon md-menu-align-target md-svg-icon="call:no-sim"></md-icon>
Option {{item}}
</md-button>
</md-menu-item>
</md-menu-content>
</md-menu>
</div>
<div layout="column" flex-sm="100" flex="33" layout-align="center center">
<p>Target mode with <code>md-offset</code></p>
<md-menu md-offset="0 -5">
<md-button aria-label="Open demo menu" class="md-icon-button" ng-click="$mdOpenMenu()">
<md-icon md-menu-origin md-svg-icon="call:ring-volume"></md-icon>
</md-button>
<md-menu-content width="4">
<md-menu-item ng-repeat="item in [1, 2, 3]">
<md-button ng-click="ctrl.announceClick($index)"> <span md-menu-align-target>Option</span> {{item}} </md-button>
</md-menu-item>
</md-menu-content>
</md-menu>
</div>
<div layout="column" flex-sm="100" flex="33" layout-align="center center">
<p><code>md-position-mode="target-right target"</code></p>
<md-menu md-position-mode="target-right target">
<md-button aria-label="Open demo menu" class="md-icon-button" ng-click="$mdOpenMenu()">
<md-icon md-menu-origin md-svg-icon="call:portable-wifi-off"></md-icon>
</md-button>
<md-menu-content width="4">
<md-menu-item ng-repeat="item in [1, 2, 3]">
<md-button ng-click="ctrl.announceClick($index)">
<p>Option {{item}}</p>
<md-icon md-menu-align-target md-svg-icon="call:portable-wifi-off"></md-icon>
</md-button>
</md-menu-item>
</md-menu-content>
</md-menu>
</div>
</div>
</div>
</div>
</div>

22 changes: 22 additions & 0 deletions src/components/menu/demoMenuPositionModes/script.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
angular.module('menuDemoPosition', ['ngMaterial'])
.config(function($mdIconProvider) {
$mdIconProvider
.iconSet("call", '/img/icons/sets/communication-icons.svg', 24)
.iconSet("social", '/img/icons/sets/social-icons.svg', 24);
})
.controller('PositionDemoCtrl', DemoCtrl);

function DemoCtrl($mdDialog) {
var vm = this;

this.announceClick = function(index) {
$mdDialog.show(
$mdDialog.alert()
.title('You clicked!')
.content('You clicked the menu item at index ' + index)
.ok('Nice')
);
};
}


7 changes: 7 additions & 0 deletions src/components/menu/demoMenuPositionModes/style.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.md-menu-demo {
padding: 24px;
}

.menu-demo-container {
min-height: 200px;
}
Loading