-
Notifications
You must be signed in to change notification settings - Fork 65
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
MenuItem
, Menu
, SubMenu
, and MenuBar
widgets
#104
Changes from all commits
cb2941a
30225d1
78d111a
f2f55aa
4c359b0
dd359ee
2938530
97135c7
487f07c
dd1bd20
9745208
00a2c82
21d8899
56b962f
3f8cb65
c149e2f
7317abf
06b0706
90dd3d5
0ec6398
7947e12
5676050
84ee217
d2f3a64
1f75bba
0fc3d80
6b13dd8
ce92ceb
cf2621b
567ecb2
afb9019
203b884
a3ce238
ab0c84d
dbf3e5c
b66fb22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,196 @@ | ||
import { createHandle } from '@dojo/core/lang'; | ||
import uuid from '@dojo/core/uuid'; | ||
import { v } from '@dojo/widget-core/d'; | ||
import { DNode } from '@dojo/widget-core/interfaces'; | ||
import ThemeableMixin, { theme, ThemeableProperties } from '@dojo/widget-core/mixins/Themeable'; | ||
import WidgetBase from '@dojo/widget-core/WidgetBase'; | ||
import * as css from './styles/menu.m.css'; | ||
import { Keys } from '../common/util'; | ||
|
||
export const enum Orientation { | ||
Horizontal, | ||
Vertical | ||
} | ||
|
||
export type RoleType = 'menu' | 'menubar'; | ||
|
||
/** | ||
* @type MenuProperties | ||
* | ||
* Properties that can be set on a Menu component. | ||
* | ||
* @property active Determines whether the menu has focus. | ||
* @property activeIndex Determines the index of the focused item. | ||
* @property disabled Determines whether the menu is disabled. | ||
* @property id The widget ID. Defaults to a random string. | ||
* @property orientation Determines whether the menu is rendered horizontally. | ||
* @property role The value to use for the menu's `role` property. Defaults to 'menu'. | ||
*/ | ||
export interface MenuProperties extends ThemeableProperties { | ||
active?: boolean; | ||
activeIndex?: number; | ||
disabled?: boolean; | ||
id?: string; | ||
orientation?: Orientation; | ||
role?: RoleType; | ||
} | ||
|
||
export const enum Operation { | ||
decrease, | ||
increase | ||
} | ||
|
||
export const MenuBase = ThemeableMixin(WidgetBase); | ||
|
||
@theme(css) | ||
export class Menu extends MenuBase<MenuProperties> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably have a custom element descriptor for all of these components. |
||
private _active = false; | ||
private _activeIndex = 0; | ||
private _domNode: HTMLElement | null; | ||
private _id: string; | ||
|
||
constructor() { | ||
/* istanbul ignore next: disregard transpiled `super`'s "else" block */ | ||
super(); | ||
// TODO: Remove once focus management is implemented. | ||
this.own(createHandle(() => { | ||
this._domNode = null; | ||
})); | ||
} | ||
|
||
render(): DNode { | ||
const { | ||
id = this._getDefaultId(), | ||
role = 'menu' | ||
} = this.properties; | ||
|
||
return v('div', { | ||
classes: this.classes.apply(this, this._getMenuClasses()), | ||
id, | ||
key: 'root', | ||
onfocusin: this._onMenuFocus, | ||
onfocusout: this._onMenuFocusOut, | ||
onkeydown: this._onMenuKeyDown, | ||
role | ||
}, this._renderChildren()); | ||
} | ||
|
||
protected onElementCreated(element: HTMLElement, key: string) { | ||
if (key === 'root') { | ||
this._domNode = element; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need to store a reference to the rendered element? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's so the widget has a way of knowing whether the current |
||
} | ||
} | ||
|
||
private _getDefaultId() { | ||
if (!this._id) { | ||
this._id = uuid(); | ||
} | ||
return this._id; | ||
} | ||
|
||
private _getDefaultOrientation(): Orientation { | ||
const { role = 'menu' } = this.properties; | ||
return role === 'menubar' ? Orientation.Horizontal : Orientation.Vertical; | ||
} | ||
|
||
private _getKeys() { | ||
const { orientation = this._getDefaultOrientation() } = this.properties; | ||
const isHorizontal = orientation === Orientation.Horizontal; | ||
|
||
return { | ||
decrease: isHorizontal ? Keys.Left : Keys.Up, | ||
increase: isHorizontal ? Keys.Right : Keys.Down, | ||
tab: Keys.Tab | ||
}; | ||
} | ||
|
||
private _getMenuClasses() { | ||
const { orientation = this._getDefaultOrientation() } = this.properties; | ||
const classes = [ css.root ]; | ||
|
||
if (orientation === Orientation.Horizontal) { | ||
classes.push(css.horizontal); | ||
} | ||
|
||
return classes; | ||
} | ||
|
||
private _isActive() { | ||
return this._active || this.properties.active; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should the |
||
} | ||
|
||
private _moveActiveIndex(operation: Operation) { | ||
const max = this.children.length; | ||
const previousIndex = this._activeIndex; | ||
this._activeIndex = operation === Operation.decrease ? | ||
previousIndex - 1 < 0 ? max - 1 : previousIndex - 1 : | ||
Math.min(previousIndex + 1, max) % max; | ||
|
||
this.invalidate(); | ||
} | ||
|
||
private _onMenuFocus() { | ||
if (!this._isActive()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in a controlled pattern I would have expected to see a |
||
this._active = true; | ||
this.invalidate(); | ||
} | ||
} | ||
|
||
private _onMenuFocusOut() { | ||
if (this._isActive()) { | ||
requestAnimationFrame(() => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do you need to requestanimationframe here? widget renders always occur on the subsequent animation frame There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When clicking within a menu, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will direct RAF usage go away if we implement some kind of focus trapping, in the same way that references to the rendered element will disappear? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As long as the focus trap jumps through any hoops it needs to track changes to |
||
const node = this._domNode as HTMLElement; | ||
if (node && !node.contains(document.activeElement)) { | ||
this._active = false; | ||
this.invalidate(); | ||
} | ||
}); | ||
} | ||
} | ||
|
||
private _onMenuKeyDown(event: KeyboardEvent) { | ||
const keys = this._getKeys(); | ||
|
||
switch (event.keyCode) { | ||
case keys.tab: | ||
event.stopPropagation(); | ||
this._active = false; | ||
this.invalidate(); | ||
break; | ||
case keys.decrease: | ||
event.preventDefault(); | ||
event.stopPropagation(); | ||
this._moveActiveIndex(Operation.decrease); | ||
break; | ||
case keys.increase: | ||
event.preventDefault(); | ||
event.stopPropagation(); | ||
this._moveActiveIndex(Operation.increase); | ||
break; | ||
} | ||
} | ||
|
||
private _onMenuItemMouseDown(index?: number) { | ||
if (typeof index === 'number') { | ||
this._activeIndex = index; | ||
} | ||
} | ||
|
||
private _renderChildren() { | ||
const { activeIndex = this._activeIndex } = this.properties; | ||
const focusIndex = this._isActive() ? activeIndex : undefined; | ||
this._activeIndex = activeIndex; | ||
|
||
this.children.filter((child: any) => child && child.properties) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are children rendered inside a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, parents will need to render to a |
||
.forEach((child: any, i) => { | ||
child.properties.active = i === focusIndex; | ||
child.properties.focusable = i === activeIndex; | ||
child.properties.index = i; | ||
child.properties.onMouseDown = this._onMenuItemMouseDown; | ||
}); | ||
|
||
return this.children; | ||
} | ||
} | ||
|
||
export default Menu; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use
next.firstCall.calledWith
where possible