Skip to content

Added ability for anchor menu items to have sub menus.#4198

Merged
dzearing merged 6 commits into
microsoft:masterfrom
dmarcey:users/demarcey/anchorMenuItems
Mar 15, 2018
Merged

Added ability for anchor menu items to have sub menus.#4198
dzearing merged 6 commits into
microsoft:masterfrom
dmarcey:users/demarcey/anchorMenuItems

Conversation

@dmarcey

@dmarcey dmarcey commented Mar 6, 2018

Copy link
Copy Markdown
Collaborator

Added prop for subMenuHoverDelay.

Pull request checklist

Description of changes

  • Allow anchor menu items to have sub menus.
  • Added a prop for sub menu hover delay.
  • Added examples for each.

@dmarcey dmarcey requested a review from joschect as a code owner March 6, 2018 21:28
@dmarcey dmarcey requested a review from dzearing March 7, 2018 18:58
*/
private _updateFocusOnMouseEvent(item: any, ev: React.MouseEvent<HTMLElement>) {
const targetElement = ev.currentTarget as HTMLElement;
const timeoutDuration = this.props.subMenuHoverDelay !== undefined ? this.props.subMenuHoverDelay : this._navigationIdleDelay;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

const { subMenuHoverDelay: timeoutDuration = this._navigationIdleDelay } = this.props;?

* menu item
*/
onItemClick?: (ev?: React.MouseEvent<HTMLElement>, item?: IContextualMenuItem) => void;
onItemClick?: (ev?: React.MouseEvent<HTMLElement>, item?: IContextualMenuItem) => boolean | void;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add comment explaining the return type?

@joschect

joschect commented Mar 8, 2018

Copy link
Copy Markdown
Contributor

@dmarcey Let's move this logic into split button. Really only split buttons should have submenu and a button.

const { contextualMenuItemAs: ChildrenRenderer = ContextualMenuItem } = this.props;

let { subMenuId } = this.state;
if (item.subMenuProps && item.subMenuProps.id) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe put this in a method since this logic is used elsewhere.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed. Thanks.

@dmarcey dmarcey force-pushed the users/demarcey/anchorMenuItems branch from 5b1c2cb to d0a7578 Compare March 15, 2018 12:34
@dzearing dzearing merged commit 1b072de into microsoft:master Mar 15, 2018
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 31, 2019
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.

Anchor Menu Items cannot contain sub menus

4 participants