Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions etc/lime-elements.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,7 @@ export namespace Components {
"commandText"?: string;
"disabled": boolean;
"hotkey"?: string;
"primaryComponent"?: ListComponent;
"showChevron": boolean;
}
// @internal (undocumented)
Expand Down Expand Up @@ -2854,6 +2855,7 @@ export namespace JSX {
"commandText"?: string;
"disabled"?: boolean;
"hotkey"?: string;
"primaryComponent"?: ListComponent;
"showChevron"?: boolean;
}

Expand Down Expand Up @@ -4191,6 +4193,7 @@ interface MenuItem<T = any> {
items?: Array<MenuItem<T> | ListSeparator> | MenuLoader;
// @internal
parentItem?: MenuItem;
primaryComponent?: ListComponent;
secondaryText?: string;
selected?: boolean;
text: string;
Expand Down
3 changes: 2 additions & 1 deletion src/components/list-item/list-item.scss
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ limel-list-item[disabled]:not([disabled='false']) {
.text,
limel-icon,
img,
.boolean-input {
.boolean-input,
limel-menu-item-meta {
opacity: 0.4;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/list-item/list-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ export class ListItemComponent implements ListItem {
const PrimaryComponent: any = primary.name;
const props = primary.props || {};

return <PrimaryComponent {...props} />;
return <PrimaryComponent {...props} disabled={this.disabled} />;
};

private renderImage = () => {
Expand Down
20 changes: 20 additions & 0 deletions src/components/list-item/menu-item-meta/menu-item-meta.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Component, Host, Prop, h } from '@stencil/core';
import { normalizeHotkeyString } from '../../../util/hotkeys';
import { ListComponent } from '../list-item.types';

/**
* Meta content for menu list items
Expand Down Expand Up @@ -47,9 +48,16 @@ export class MenuItemMeta {
@Prop()
public showChevron = false;

/**
* Optional primary component to render before other meta content
*/
@Prop()
public primaryComponent?: ListComponent;

public render() {
return (
<Host>
{this.renderPrimaryComponent()}
{this.renderCommandText()}
{this.renderBadge()}
{this.renderChevron()}
Expand Down Expand Up @@ -80,6 +88,18 @@ export class MenuItemMeta {
return <limel-badge label={String(this.badge)} />;
}

private renderPrimaryComponent() {
const primary = this.primaryComponent;
if (!primary?.name) {
return;
}

const PrimaryComponent: any = primary.name;
const props = primary.props || {};

return <PrimaryComponent {...props} disabled={this.disabled} />;
}

private renderChevron() {
if (!this.showChevron) {
return;
Expand Down
26 changes: 14 additions & 12 deletions src/components/menu-list/menu-list-renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,20 @@ export class MenuListRenderer {
!!item.hotkey ||
!!item.commandText;

const primaryComponent = hasMeta
? {
name: 'limel-menu-item-meta',
props: {
commandText: item.commandText,
hotkey: item.hotkey,
disabled: !!item.disabled,
badge: item.badge,
showChevron: hasSubMenu,
},
}
: undefined;
const primaryComponent =
hasMeta || item.primaryComponent
? {
name: 'limel-menu-item-meta',
props: {
commandText: item.commandText,
hotkey: item.hotkey,
disabled: !!item.disabled,
badge: item.badge,
showChevron: hasSubMenu,
primaryComponent: item.primaryComponent,
},
}
: undefined;

const key = (item as any).id ?? `item-${index}`;
const classNames = {
Expand Down
77 changes: 77 additions & 0 deletions src/components/menu/examples/menu-primary-component.tsx
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';
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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

fd -a "menu-primary-component.tsx" src/components/menu/examples/

Repository: Lundalogik/lime-elements

Length of output: 142


🏁 Script executed:

# Check if file exists and examine the relevant lines
if [ -f "src/components/menu/examples/menu-primary-component.tsx" ]; then
  echo "=== File Content with Line Numbers ==="
  cat -n "src/components/menu/examples/menu-primary-component.tsx" | head -80
fi

Repository: Lundalogik/lime-elements

Length of output: 2882


Wrap the example output in <Host>.

Returning an array literal from the render method violates the repo's StencilJS pattern. Use <Host> to wrap multiple child elements instead, which aligns with the codebase and removes the missing-key warnings.

♻️ Suggested change
-import { Component, h, State } from '@stencil/core';
+import { Component, h, Host, State } from '@stencil/core';
…
     public render() {
-        return [
-            <limel-menu items={this.items} onSelect={this.handleSelect}>
-                <limel-button label="Tasks" slot="trigger" />
-            </limel-menu>,
-            <limel-example-value value={this.lastSelectedItem} />,
-        ];
+        return (
+            <Host>
+                <limel-menu items={this.items} onSelect={this.handleSelect}>
+                    <limel-button label="Tasks" slot="trigger" />
+                </limel-menu>
+                <limel-example-value value={this.lastSelectedItem} />
+            </Host>
+        );
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/menu/examples/menu-primary-component.tsx` at line 2, The
render method currently returns an array literal; update it to import and use
Host from '@stencil/core' and wrap the multiple child elements inside <Host>
instead of returning an array (modify the existing import line to include Host
and change the render implementation in the component class's render method to
return a single <Host> element containing the children). Ensure no array is
returned and that all previous child elements are placed as direct children of
Host.


/**
* 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

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Member 'items' is never reassigned; mark it as `readonly`.

See more on https://sonarcloud.io/project/issues?id=Lundalogik_lime-elements&issues=AZ1NSRoRLJvKeqHjdqfs&open=AZ1NSRoRLJvKeqHjdqfs&pullRequest=3994
{
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

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Missing "key" prop for element in array

See more on https://sonarcloud.io/project/issues?id=Lundalogik_lime-elements&issues=AZ1NSRoRLJvKeqHjdqft&open=AZ1NSRoRLJvKeqHjdqft&pullRequest=3994
<limel-example-value value={this.lastSelectedItem} />,

Check warning on line 70 in src/components/menu/examples/menu-primary-component.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Missing "key" prop for element in array

See more on https://sonarcloud.io/project/issues?id=Lundalogik_lime-elements&issues=AZ1NSRoRLJvKeqHjdqfu&open=AZ1NSRoRLJvKeqHjdqfu&pullRequest=3994
];
}

private handleSelect = (event: LimelMenuCustomEvent<MenuItem>) => {

Check warning on line 74 in src/components/menu/examples/menu-primary-component.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Member 'handleSelect' is never reassigned; mark it as `readonly`.

See more on https://sonarcloud.io/project/issues?id=Lundalogik_lime-elements&issues=AZ1NSRoRLJvKeqHjdqfv&open=AZ1NSRoRLJvKeqHjdqfv&pullRequest=3994
this.lastSelectedItem = event.detail.text;
};
}
1 change: 1 addition & 0 deletions src/components/menu/menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const DEFAULT_ROOT_BREADCRUMBS_ITEM: BreadcrumbsItem = {
* @exampleComponent limel-example-menu-surface-width
* @exampleComponent limel-example-menu-separators
* @exampleComponent limel-example-menu-icons
* @exampleComponent limel-example-menu-primary-component
* @exampleComponent limel-example-menu-badge-icons
* @exampleComponent limel-example-menu-grid
* @exampleComponent limel-example-menu-secondary-text
Expand Down
11 changes: 11 additions & 0 deletions src/components/menu/menu.types.ts
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.
Expand Down Expand Up @@ -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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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
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.

🛠️ Refactor suggestion | 🟠 Major

Clarify that menu primaryComponent is trailing, display-only content.

This public prop is routed through limel-menu-item-meta, while the row itself still behaves as a single menuitem. Please spell out here that it renders in the menu item's meta area and must stay non-interactive; otherwise consumers can assume list-item-like leading placement or nest focusable controls that break menu keyboarding. Please align the example copy with that contract too.

✏️ 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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* 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;
/**
* 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.
*/
primaryComponent?: ListComponent;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/menu/menu.types.ts` around lines 125 - 133, Update the JSDoc
for the primaryComponent prop to clarify that primaryComponent is rendered in
the menu item's meta/trailing area (routed via limel-menu-item-meta), is
display-only and must remain non-interactive so the row still behaves as a
single ARIA menuitem; mention that it must not contain focusable controls or
assume leading/list-item placement, and update the
limel-example-menu-primary-component example text to reflect this contract.

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.

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 needs

For 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.
*/
Expand Down
Loading