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

feat(menu, menu-item): Adds menu & menu-item components. #6901

Merged

Conversation

anveshmekala
Copy link
Contributor

@anveshmekala anveshmekala commented May 2, 2023

Related Issue: #6531

Summary

This PR adds calcite-menu & calcite-menu-item components.

Co-authored by @macandcheese
Extracted from #6829

calcite-nav-menu

Usage

Basic

<calcite-menu> <calcite-menu-item id="Nature" text="Nature"> </calcite-menu-item></calcite-menu>

Properties

Property Attribute Description Type Default
label (required) label Specifies accessible label for the component. string undefined
layout layout Specifies the layout of the component. "horizontal" | "vertical" "horizontal"
messageOverrides -- Use this property to override individual strings used by the component. { more?: string; } undefined

#Methods

setFocus() => Promise<void>

Sets focus on the component's first focusable element.

Returns

Type: Promise<void>


calcite-nav-menu-item

Usage

Basic

<calcite-menu> <calcite-menu-item id="Nature" text="Nature"> </calcite-menu-item></calcite-menu>

Nested-With-Href

Nested SubMenu with href.

<calcite-menu>
  <calcite-menu-item id="Nature" text="Nature" href="#">
    <calcite-menu-item id="Mountains" text="Mountains" slot="sub-menu-item"> </calcite-menu-item>
  </calcite-menu-item>
</calcite-menu>

Properties

Property Attribute Description Type Default
active active When true, the component is highlighted. boolean undefined
breadcrumb breadcrumb When true, the component displays a visual indication of breadcrumb boolean undefined
href href Specifies the URL destination of the component, which can be set as an absolute or relative path. string undefined
iconEnd icon-end Specifies an icon to display at the end of the component. string undefined
iconFlipRtl icon-flip-rtl Displays the iconStart and/or iconEnd as flipped when the element direction is right-to-left ("rtl"). "both" | "end" | "start" undefined
iconStart icon-start Specifies an icon to display at the start of the component. string undefined
label (required) label Specifices accessible name for the component. string undefined
open open When true, the menu item will display any slotted calcite-menu-item in an open overflow menu boolean false
rel rel Defines the relationship between the href value and the current document. string undefined
target target Specifies where to open the linked document defined in the href property. string undefined
text text Specifies the text to display. string undefined

Events

Event Description Type
calciteMenuItemSelect Emits when user selects the component. CustomEvent<void>

Methods

setFocus() => Promise<void>

Sets focus on the component.

Returns

Type: Promise<void>


Built with StencilJS

@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label May 2, 2023
@anveshmekala anveshmekala marked this pull request as ready for review May 5, 2023 01:25
@anveshmekala anveshmekala requested a review from a team as a code owner May 5, 2023 01:25
@anveshmekala anveshmekala added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label May 5, 2023
@anveshmekala anveshmekala marked this pull request as draft May 5, 2023 14:52
@anveshmekala anveshmekala marked this pull request as ready for review May 5, 2023 15:49
@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 5, 2023
@anveshmekala anveshmekala requested a review from geospatialem May 5, 2023 15:51
@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 5, 2023
@anveshmekala anveshmekala requested a review from SkyeSeitz May 5, 2023 16:38
@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 9, 2023
@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 9, 2023
Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

👍

src/components/menu/menu.stories.ts Show resolved Hide resolved
[CSS.isRtl]: dir === "rtl",
[CSS.dropdownVertical]: this.topLevelMenuLayout === "vertical"
}}
label="Submenu"
Copy link
Member

Choose a reason for hiding this comment

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

This label string still needs to be translated.

@anveshmekala
Copy link
Contributor Author

This label string still needs to be translated

Submitted the request for translations yesterday and also added the base translations strings in #6938 . Once the generate build is back we can add the t9n interface.

@anveshmekala anveshmekala changed the title feat(menu,menu-item): Adds menu & menu-item components. feat(menu, menu-item): Adds menu & menu-item components. May 9, 2023
@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 9, 2023
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

This is looking great, @anveshmekala @macandcheese! 🧑‍🍳

Only the prop reflection consistency and delegatesFocus concerns need to be addressed before merging. The rest can be tackled in follow-up PRs.

src/components/menu-item/Usage/Basic.md Outdated Show resolved Hide resolved
src/components/menu-item/Usage/Nested-With-Href.md Outdated Show resolved Hide resolved
stencil.config.ts Outdated Show resolved Hide resolved
t9nmanifest.txt Show resolved Hide resolved
src/utils/dom.ts Outdated Show resolved Hide resolved
src/components/menu-item/menu-item.tsx Outdated Show resolved Hide resolved
src/components/menu-item/menu-item.tsx Outdated Show resolved Hide resolved
src/components/menu-item/menu-item.tsx Outdated Show resolved Hide resolved
src/components/menu-item/menu-item.tsx Outdated Show resolved Hide resolved

it("is focusable", () => focusable("calcite-menu-item"));

//todo : debug why calcite-menu-item requires calite-menu as parent for the click to emit event in test.
Copy link
Member

Choose a reason for hiding this comment

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

This is being caused by the click-handling element (anchor) not stretching along with its host and therefore not being in the center when the click happens (E2EElement.click() scrolls and clicks on the center of the element). If you set inline-size: 100% on the anchor, it fixes the test, but not sure if that will cause layout issues (design-wise).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense. we can mark inline-size:100% for anchor element and the design will be similar.

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

This is looking great, @anveshmekala @macandcheese! 🧑‍🍳

Only the prop reflection consistency and delegatesFocus concerns need to be addressed before merging. The rest can be tackled in follow-up PRs.

@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 12, 2023
@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 12, 2023
@anveshmekala anveshmekala merged commit 0990bf6 into master May 12, 2023
@anveshmekala anveshmekala deleted the anveshmekala/6531-feat-add-menu-and-menuItem-components branch May 12, 2023 22:21
@github-actions github-actions bot added this to the 2023 May Priorities milestone May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues tied to a new feature or request. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants