-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Menu : deprecate props.mode / always support full capabilities + icons #18703
base: main
Are you sure you want to change the base?
Changes from all commits
8026393
4280f6a
8f84b06
757731a
3d442ff
81ca577
c3b7e95
bdc49c6
7aa8d7f
c03abb7
7b08f45
27804f5
404c599
f1e2e85
17aaf36
d4cc6a0
0eab875
f10cb0a
380b2bc
840b68d
c2f2a1c
ac7e77e
ed4d188
5d2763c
b46ba26
a5ba6ac
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,15 @@ | ||
/** | ||
* @license | ||
* | ||
* Copyright IBM Corp. 2024 | ||
* | ||
* This source code is licensed under the Apache-2.0 license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
import './menu'; | ||
import './menu-item'; | ||
import './menu-item-divider'; | ||
import './menu-item-group'; | ||
import './menu-item-selectable'; | ||
import './menu-item-radio-group'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/** | ||
* @license | ||
* | ||
* Copyright IBM Corp. 2024 | ||
* | ||
* This source code is licensed under the Apache-2.0 license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
import { createContext } from '@lit/context'; | ||
export type StateType = { | ||
isRoot: boolean; | ||
hasIcons: boolean; | ||
hasSelectableItems: boolean; | ||
size: 'xs' | 'sm' | 'md' | 'lg' | null; | ||
items: any[]; | ||
requestCloseRoot: (e: KeyboardEvent) => void; | ||
}; | ||
|
||
export const menuDefaultState: StateType = { | ||
isRoot: true, | ||
hasIcons: true, | ||
hasSelectableItems: false, | ||
size: null, | ||
items: [], | ||
requestCloseRoot: () => {}, | ||
}; | ||
|
||
export const MenuContext = createContext<StateType>('myData'); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
/** | ||
* @license | ||
* | ||
* Copyright IBM Corp. 2024 | ||
* | ||
* This source code is licensed under the Apache-2.0 license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
import { LitElement, html } from 'lit'; | ||
import { prefix } from '../../globals/settings'; | ||
import styles from './menu-item.scss?lit'; | ||
import { carbonElement as customElement } from '../../globals/decorators/carbon-element'; | ||
/** | ||
* Menu Item. | ||
* | ||
* @element cds-menu-item-divider | ||
*/ | ||
@customElement(`${prefix}-menu-item-divider`) | ||
class CDSmenuItemDivider extends LitElement { | ||
render() { | ||
return html` | ||
<li class="${prefix}--menu-item-divider" role="separator"></li> | ||
`; | ||
} | ||
static styles = styles; // `styles` here is a `CSSResult` generated by custom Vite loader | ||
} | ||
export default CDSmenuItemDivider; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
/** | ||
* @license | ||
* | ||
* Copyright IBM Corp. 2024 | ||
* | ||
* This source code is licensed under the Apache-2.0 license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
import { LitElement, html } from 'lit'; | ||
import { property } from 'lit/decorators.js'; | ||
import { prefix } from '../../globals/settings'; | ||
import styles from './menu-item.scss?lit'; | ||
import { carbonElement as customElement } from '../../globals/decorators/carbon-element'; | ||
/** | ||
* Menu Item. | ||
* | ||
* @element cds-menu-item-group | ||
*/ | ||
@customElement(`${prefix}-menu-item-group`) | ||
class CDSmenuItemGroup extends LitElement { | ||
/** | ||
* Label for the menu item. | ||
*/ | ||
@property({ type: String }) | ||
label; | ||
|
||
render() { | ||
const { label } = this; | ||
return html` | ||
<li class="${prefix}--menu-item-group" role="none"> | ||
<ul role="group" aria-label="${label}"> | ||
<slot></slot> | ||
</ul> | ||
</li> | ||
`; | ||
} | ||
static styles = styles; // `styles` here is a `CSSResult` generated by custom Vite loader | ||
} | ||
export default CDSmenuItemGroup; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
/** | ||
* @license | ||
* | ||
* Copyright IBM Corp. 2024 | ||
* | ||
* This source code is licensed under the Apache-2.0 license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
import { LitElement, html } from 'lit'; | ||
import { property } from 'lit/decorators.js'; | ||
import { prefix } from '../../globals/settings'; | ||
import styles from './menu-item.scss?lit'; | ||
import { carbonElement as customElement } from '../../globals/decorators/carbon-element'; | ||
import { ChangeEventHandler } from 'react'; | ||
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. shouldn't have to import from react if we follow the comment I made below in |
||
import { consume } from '@lit/context'; | ||
import { MenuContext } from './menu-context'; | ||
|
||
/** | ||
* Menu Item. | ||
* | ||
* @element cds-menu-item-radio-group | ||
*/ | ||
@customElement(`${prefix}-menu-item-radio-group`) | ||
class CDSmenuItemRadioGroup extends LitElement { | ||
// @provide({ context: MenuContext }) | ||
@consume({ context: MenuContext }) | ||
context; | ||
|
||
/** | ||
* Label for the menu item radio group. | ||
*/ | ||
@property({ type: String }) | ||
label; | ||
/** | ||
* List of items in the radio group. | ||
*/ | ||
@property({ type: Array }) | ||
items = []; | ||
|
||
/** | ||
* Selected item in the radio group. | ||
*/ | ||
@property({ type: String, attribute: true }) | ||
selectedItem; | ||
|
||
/** | ||
* List of items in the radio group. | ||
*/ | ||
@property() | ||
itemToString?: (item: Array<String | number>) => String; | ||
/** | ||
* Provide an optional function to be called when the selection state changes. | ||
*/ | ||
@property() | ||
onChange?: ChangeEventHandler; | ||
|
||
firstUpdated(): void { | ||
this.context.updateFromChild?.({ hasSelectableItems: true }); | ||
} | ||
|
||
_handleClick = (item, e) => { | ||
this.selectedItem = item; | ||
if (this.onChange) { | ||
this.onChange(e); | ||
} | ||
}; | ||
render() { | ||
const defaultItemToString = (item) => item.toString(); | ||
const { | ||
label, | ||
items, | ||
itemToString = defaultItemToString, | ||
selectedItem, | ||
_handleClick: handleClick, | ||
} = this; | ||
return html` | ||
<li class="${prefix}--menu-item-radio-group" role="none"> | ||
<ul role="group" aria-label="${label}"> | ||
${items.map( | ||
(item) => html` | ||
<cds-menu-item | ||
label="${itemToString(item)}" | ||
role="menuitemradio" | ||
aria-checked="${item === selectedItem}" | ||
.onClick="${(e) => { | ||
handleClick(item, e); | ||
}}"></cds-menu-item> | ||
` | ||
)} | ||
</ul> | ||
</li> | ||
`; | ||
} | ||
static styles = styles; // `styles` here is a `CSSResult` generated by custom Vite loader | ||
} | ||
export default CDSmenuItemRadioGroup; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
/** | ||
* @license | ||
* | ||
* Copyright IBM Corp. 2019, 2023 | ||
* | ||
* This source code is licensed under the Apache-2.0 license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
import { LitElement, html } from 'lit'; | ||
import { property } from 'lit/decorators.js'; | ||
import { prefix } from '../../globals/settings'; | ||
import styles from './menu-item.scss?lit'; | ||
import { carbonElement as customElement } from '../../globals/decorators/carbon-element'; | ||
import { consume } from '@lit/context'; | ||
import { MenuContext } from './menu-context'; | ||
/** | ||
* Menu Item. | ||
* | ||
* @element cds-menu-item | ||
*/ | ||
@customElement(`${prefix}-menu-item-selectable`) | ||
class CDSmenuItemSelectable extends LitElement { | ||
@consume({ context: MenuContext }) | ||
context; | ||
/** | ||
* Label for the menu item selectable. | ||
*/ | ||
@property({ type: String }) | ||
label; | ||
/** | ||
* Whether the menu item is selected or not. | ||
*/ | ||
@property({ type: Boolean }) | ||
selected = false; | ||
|
||
/** | ||
* Provide an optional function to be called when the selection state changes. | ||
*/ | ||
onChange?; | ||
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 usually let adopters tap into event changes by emitting a custom event like so in On click of the close icon in
|
||
|
||
/** | ||
* Sets the menu item's icon. | ||
*/ | ||
@property() | ||
renderIcon?: () => void; | ||
|
||
_handleClick = (e) => { | ||
this.selected = !this.selected; | ||
if (this.onChange) { | ||
this.onChange(e); | ||
} | ||
}; | ||
firstUpdated(): void { | ||
this.context.updateFromChild?.({ hasSelectableItems: true }); | ||
} | ||
render() { | ||
const { label, selected, _handleClick: handleClick } = this; | ||
|
||
return html` | ||
<cds-menu-item | ||
label="${label}" | ||
class="${prefix}--menu-item-selectable--selected" | ||
role="menuitemcheckbox" | ||
aria-checked="${selected}" | ||
.renderIcon=${this.renderIcon || undefined} | ||
.onClick="${handleClick}"></cds-menu-item> | ||
`; | ||
} | ||
static styles = styles; // `styles` here is a `CSSResult` generated by custom Vite loader | ||
} | ||
export default CDSmenuItemSelectable; |
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.
In general we try to avoid having unessarry rendering of html if we can apply the attributes (ie class and role) to the the custom element itself.
Here is an example:
https://github.com/carbon-design-system/carbon/blob/main/packages/web-components/src/components/ui-shell/side-nav-divider.ts
We could additionally remove the
"${prefix}--menu-item-divider"
class since we can just extend the styles like so: https://github.com/carbon-design-system/carbon/blob/main/packages/web-components/src/components/ui-shell/side-nav.scss#L47When a adopter then uses
cds-menu-item-divider
then they can apply a class like:and then they would be able to apply custom styles to the same level as the
menu-item-divider
styles vs the way it's implemented now the adopter would have a custom class at a higher level then the<li class="${prefix}--menu-item-divider"/>
levelThis should be applied to all the components and not just this one