-
Notifications
You must be signed in to change notification settings - Fork 17
feat(menu): use primaryComponent from the limel-list #3994
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
base: main
Are you sure you want to change the base?
Changes from all commits
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,77 @@ | ||
| import { LimelMenuCustomEvent, MenuItem } from '@limetech/lime-elements'; | ||
| import { Component, h, State } from '@stencil/core'; | ||
|
|
||
| /** | ||
| * With a primary component | ||
| * | ||
| * Menu items can render a custom primary component using the | ||
| * `primaryComponent` prop, just like list items. | ||
| * | ||
| * The component is rendered before the item's text. | ||
| */ | ||
| @Component({ | ||
| tag: 'limel-example-menu-primary-component', | ||
| shadow: true, | ||
| }) | ||
| export class MenuPrimaryComponentExample { | ||
| @State() | ||
| private lastSelectedItem: string; | ||
|
|
||
| private items: MenuItem[] = [ | ||
|
Check warning on line 20 in src/components/menu/examples/menu-primary-component.tsx
|
||
| { | ||
| text: 'Upload file', | ||
| secondaryText: 'Completed', | ||
| primaryComponent: { | ||
| name: 'limel-circular-progress', | ||
| props: { | ||
| value: 10, | ||
| maxValue: 10, | ||
| suffix: '%', | ||
| displayPercentageColors: true, | ||
| size: 'small', | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| text: 'Sync contacts', | ||
| secondaryText: 'In progress…', | ||
| primaryComponent: { | ||
| name: 'limel-circular-progress', | ||
| props: { | ||
| value: 4, | ||
| maxValue: 10, | ||
| suffix: '%', | ||
| displayPercentageColors: true, | ||
| size: 'small', | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| text: 'Generate report', | ||
| secondaryText: 'Just started', | ||
| primaryComponent: { | ||
| name: 'limel-circular-progress', | ||
| props: { | ||
| value: 1, | ||
| maxValue: 10, | ||
| suffix: '%', | ||
| displayPercentageColors: true, | ||
| size: 'small', | ||
| }, | ||
| }, | ||
| }, | ||
| ]; | ||
|
|
||
| public render() { | ||
| return [ | ||
| <limel-menu items={this.items} onSelect={this.handleSelect}> | ||
| <limel-button label="Tasks" slot="trigger" /> | ||
| </limel-menu>, | ||
|
Check warning on line 69 in src/components/menu/examples/menu-primary-component.tsx
|
||
| <limel-example-value value={this.lastSelectedItem} />, | ||
|
Check warning on line 70 in src/components/menu/examples/menu-primary-component.tsx
|
||
| ]; | ||
| } | ||
|
|
||
| private handleSelect = (event: LimelMenuCustomEvent<MenuItem>) => { | ||
|
Check warning on line 74 in src/components/menu/examples/menu-primary-component.tsx
|
||
| this.lastSelectedItem = event.detail.text; | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { ListSeparator } from '../../global/shared-types/separator.types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Icon } from '../../global/shared-types/icon.types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Color } from '../../global/shared-types/color.types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { ListComponent } from '../list-item/list-item.types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * The direction in which the menu should open. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -121,6 +122,16 @@ export interface MenuItem<T = any> { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| badge?: number | string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Component used to render additional content in the menu item. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * When the menu item is disabled, the `disabled` prop is automatically | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * propagated to the primary component. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * See `limel-example-menu-primary-component` for usage. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| primaryComponent?: ListComponent; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+125
to
+133
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. I think that we maybe should add an example as mr Robot here says, what do you think?
Comment on lines
+125
to
+133
Contributor
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. 🛠️ Refactor suggestion | 🟠 Major Clarify that menu This public prop is routed through ✏️ Suggested JSDoc update /**
- * Component used to render additional content in the menu item.
+ * Component used to render additional trailing meta content in the menu item.
+ *
+ * It is rendered together with the menu item's meta content, before
+ * shortcut text, badges, and submenu chevrons.
+ *
+ * This content must be presentational only. Do not pass focusable or
+ * interactive components, since the row itself remains the single
+ * `menuitem`.
*
* When the menu item is disabled, the `disabled` prop is automatically
* propagated to the primary component.
*
* See `limel-example-menu-primary-component` for usage.
*/📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
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. I had a chat with @LucyChyzhova earlier today. Mayeb you have already talked about it. But just to make sure you have heard my feedback 😄 : I don't think it's a good idea to make the interface and APIs of the menu more complicated than it already is. The primary component slot is already being used by the menu itself, for other internal things (such as hotkeys, command text, caret, badge). It's really hard to motivate having additional stuff here, decided by the consumer. They need to coexist and work well. The UI will be really hard to maintain if you add this, because we have to deal with many unforeseen scenarios. Please don't merge this PR. 🙏 if this was really a must have, a better choice would be perhaps introducing a new slot. But I wouldn't do that neither. Regarding your design needsFor your particular use case, a badge would be enough, which already exists. No need for more features like this really, to address that need. But I would also question that design. I think you have better choices for what you wanna achieve 🤗 |
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Value of the menu item. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
🧩 Analysis chain
🏁 Script executed:
fd -a "menu-primary-component.tsx" src/components/menu/examples/Repository: Lundalogik/lime-elements
Length of output: 142
🏁 Script executed:
Repository: Lundalogik/lime-elements
Length of output: 2882
Wrap the example output in
<Host>.Returning an array literal from the
rendermethod violates the repo's StencilJS pattern. Use<Host>to wrap multiple child elements instead, which aligns with the codebase and removes the missing-keywarnings.♻️ Suggested change
🤖 Prompt for AI Agents