Add limel-list-item & menu-item-meta private components / refactor & improve list renderers / remove dependency to MDC's styles for list & list-items
#3628
Conversation
Kiarokh
added
the
maintenance
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughIntroduces a new limel-list-item component and supporting styles. Refactors list and menu-list renderers to delegate item rendering to limel-list-item, updates tests to assert reflected attributes, and adjusts styles/partials accordingly. Adds a menu-item-meta subcomponent. Provides multiple example components and SCSS demonstrating icons, actions, selection, primary components, pictures, and layout options. Minor deprecation text updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant List as limel-list / limel-menu-list
participant Item as limel-list-item
participant Menu as limel-menu (actions)
participant App as Consumer
User->>List: Focus/Click on item
List->>Item: Render item props (text, icon, type, selected, actions, ...)
User->>Item: Click / Enter / Space
alt Item disabled
Item-->>User: No action
else Selectable (option/radio/checkbox)
Item->>Item: Toggle selected
Item-->>List: emit("interact", { selected, item })
List-->>App: propagate change (selection state)
else Non-selectable
Item-->>List: emit("interact", { selected: false, item })
List-->>App: propagate interaction
end
opt Actions menu provided
User->>Item: Click action trigger
Item->>Menu: Open with actions
User->>Menu: Choose action
Menu-->>App: action event with chosen item
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3628/ |
Kiarokh
force-pushed
the
list-item
branch
2 times, most recently
from
2af18fa to
cd1dc2e
Compare
Kiarokh
changed the title
@private component
Kiarokh
force-pushed
the
list-item
branch
8 times, most recently
from
9c618e2 to
f139cae
Compare
Kiarokh
force-pushed
the
list-item
branch
4 times, most recently
from
0065952 to
4a753da
Compare
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 49
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/components/list/partial-styles/custom-styles.scss (1)
11-24: Zebra-stripe CSS variables look swapped (even uses “odd” var and vice versa)The var names suggest “odd” colors for odd rows, but they’re applied to the opposite nth-child. If that’s unintentional, swap them.
- &:nth-child(even) { - background-color: var( - --list-background-color-of-odd-interactive-items, - rgb(var(--contrast-200)) - ); - } - &:nth-child(odd) { - background-color: var( - --list-background-color-of-even-interactive-items, - rgb(var(--contrast-100)) - ); - } + &:nth-child(even) { + background-color: var( + --list-background-color-of-even-interactive-items, + rgb(var(--contrast-200)) + ); + } + &:nth-child(odd) { + background-color: var( + --list-background-color-of-odd-interactive-items, + rgb(var(--contrast-100)) + ); + }src/components/menu-list/menu-list.e2e.ts (1)
58-59: Remove unnecessary microtask delay; waitForChanges is sufficientThe extra
setTimeout(0)shouldn’t be needed afterawait page.waitForChanges()and will slow tests.- await new Promise((resolve) => setTimeout(resolve, 0));src/components/list/list.e2e.ts (1)
1-1: Replace absolute imports with relative, type-only importsPer repo guidelines, non-example TS/TSX files must not import from the package’s public API. Using relative, type-only imports for internal types prevents build issues and removes unnecessary runtime dependencies.
• File src/components/list/list.e2e.ts, line 1:
- import { ListItem, ListSeparator } from '@limetech/lime-elements'; + import type { ListItem } from './list-item.types'; + import type { ListSeparator } from '../../global/shared-types/separator.types';To catch similar violations across the repo, run:
rg -nP -g '*.ts' -g '*.tsx' -g '!src/components/**/examples/**' -e "from\s+['\"]@limetech/lime-elements['\"]"src/components/menu-list/menu-list-renderer.tsx (1)
29-33: AddmaxLinesSecondaryTexttoMenuListRendererConfigand make it configurableThe renderer is currently hard-coding
--maxLinesSecondaryText: '2', but theMenuListRendererConfiginterface doesn’t expose this option. This needs a mandatory refactor to restore configurability and avoid breaking consumers.• In
src/components/menu-list/menu-list-renderer.tsx(around line 32), replace the hard-coded style with a dynamic config fallback
• Insrc/components/menu-list/menu-list-renderer-config.ts, extend the interface withmaxLinesSecondaryText?: number
• Document the new option in the CHANGELOG and note that the default remains2(breaking change)Suggested diff:
--- a/src/components/menu-list/menu-list-renderer.tsx @@ export class MenuListRenderer { - <ul - class="mdc-deprecated-list" - role="menu" - aria-orientation="vertical" - style={{ '--maxLinesSecondaryText': '2' }} - > + <ul + class="mdc-deprecated-list" + role="menu" + aria-orientation="vertical" + style={{ + '--maxLinesSecondaryText': String( + this.config.maxLinesSecondaryText ?? 2 + ), + }} + >--- a/src/components/menu-list/menu-list-renderer-config.ts @@ export interface MenuListRendererConfig { iconSize?: IconSize; + /** Max number of lines for secondary text before truncation */ + maxLinesSecondaryText?: number; }src/components/list/list.scss (1)
26-38: UL reset looks good; consider exposing list margin as a public variable
--list-marginis currently internal. If consumers need to align lists with surrounding content, consider documenting it as public; otherwise prefix with--limel-list-marginto clarify scope.
♻️ Duplicate comments (1)
src/components/list-item/list-item.tsx (1)
44-44: API extractor error needs resolution.Based on the past review comment, the
api-extractorfails to analyzeListItemInterfaceexport. This issue may persist and needs to be addressed before merging.#!/bin/bash # Verify if the API extractor issue has been resolved # Check if ListItemInterface is properly exported from list-item.types.ts fd -e ts "list-item.types" --exec cat {} \; | head -20
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (32)
src/components/list-item/examples/list-item-actions.tsx(1 hunks)src/components/list-item/examples/list-item-basic.scss(1 hunks)src/components/list-item/examples/list-item-basic.tsx(1 hunks)src/components/list-item/examples/list-item-checkbox.tsx(1 hunks)src/components/list-item/examples/list-item-command-text.tsx(1 hunks)src/components/list-item/examples/list-item-icon-size.tsx(1 hunks)src/components/list-item/examples/list-item-icon.tsx(1 hunks)src/components/list-item/examples/list-item-interactive.scss(1 hunks)src/components/list-item/examples/list-item-interactive.tsx(1 hunks)src/components/list-item/examples/list-item-multiple-lines.tsx(1 hunks)src/components/list-item/examples/list-item-pictures.tsx(1 hunks)src/components/list-item/examples/list-item-primary-component.tsx(1 hunks)src/components/list-item/examples/list-item-radio.tsx(1 hunks)src/components/list-item/list-item.scss(1 hunks)src/components/list-item/list-item.tsx(1 hunks)src/components/list-item/menu-item-meta/menu-item-meta.scss(1 hunks)src/components/list-item/menu-item-meta/menu-item-meta.tsx(1 hunks)src/components/list/examples/list-badge-icons.scss(1 hunks)src/components/list/list-item.types.ts(2 hunks)src/components/list/list-renderer.tsx(2 hunks)src/components/list/list.e2e.ts(5 hunks)src/components/list/list.scss(3 hunks)src/components/list/list.tsx(1 hunks)src/components/list/partial-styles/_has-grid-layout.scss(2 hunks)src/components/list/partial-styles/_static-actions.scss(2 hunks)src/components/list/partial-styles/custom-styles.scss(2 hunks)src/components/list/partial-styles/enable-multiline-text.scss(0 hunks)src/components/menu-list/menu-list-renderer.tsx(2 hunks)src/components/menu-list/menu-list.e2e.ts(3 hunks)src/components/menu-list/menu-list.scss(1 hunks)src/components/menu-list/menu-list.tsx(1 hunks)src/components/menu/examples/menu-badge-icons.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/list/partial-styles/enable-multiline-text.scss
🧰 Additional context used
📓 Path-based instructions (6)
src/components/**/*.scss
📄 CodeRabbit inference engine (.cursor/rules/css-class-names-in-components-using-shadow-dom.mdc)
Do not use BEM-style class names in CSS for components, as components use shadow-DOM.
Files:
src/components/list-item/examples/list-item-interactive.scsssrc/components/list/partial-styles/_has-grid-layout.scsssrc/components/list/partial-styles/custom-styles.scsssrc/components/list/examples/list-badge-icons.scsssrc/components/list-item/examples/list-item-basic.scsssrc/components/menu-list/menu-list.scsssrc/components/list-item/list-item.scsssrc/components/list-item/menu-item-meta/menu-item-meta.scsssrc/components/list/partial-styles/_static-actions.scsssrc/components/list/list.scss
**/*.{tsx,scss}
⚙️ CodeRabbit configuration file
**/*.{tsx,scss}: Almost all our components use shadow-DOM. Therefore, we have no need of BEM-style class names in our CSS.
Files:
src/components/list-item/examples/list-item-interactive.scsssrc/components/list/list.tsxsrc/components/list/partial-styles/_has-grid-layout.scsssrc/components/list-item/examples/list-item-icon.tsxsrc/components/list-item/examples/list-item-interactive.tsxsrc/components/list/partial-styles/custom-styles.scsssrc/components/list/examples/list-badge-icons.scsssrc/components/list-item/examples/list-item-radio.tsxsrc/components/list-item/list-item.tsxsrc/components/list-item/examples/list-item-icon-size.tsxsrc/components/list-item/menu-item-meta/menu-item-meta.tsxsrc/components/list-item/examples/list-item-basic.scsssrc/components/list-item/examples/list-item-command-text.tsxsrc/components/list-item/examples/list-item-pictures.tsxsrc/components/list-item/examples/list-item-primary-component.tsxsrc/components/menu-list/menu-list.scsssrc/components/list-item/list-item.scsssrc/components/list-item/examples/list-item-checkbox.tsxsrc/components/menu-list/menu-list.tsxsrc/components/list/list-renderer.tsxsrc/components/list-item/menu-item-meta/menu-item-meta.scsssrc/components/list-item/examples/list-item-basic.tsxsrc/components/list-item/examples/list-item-actions.tsxsrc/components/menu-list/menu-list-renderer.tsxsrc/components/menu/examples/menu-badge-icons.tsxsrc/components/list-item/examples/list-item-multiple-lines.tsxsrc/components/list/partial-styles/_static-actions.scsssrc/components/list/list.scss
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/wrap-multiple-jsx-elements-in-host-not-in-array.mdc)
When returning multiple JSX elements from the
rendermethod, never wrap them in an array literal. Instead, always wrap them in the special<Host>element. When there is already a single top-level element, it does not have to be wrapped in<Host>.
Files:
src/components/list/list.tsxsrc/components/list-item/examples/list-item-icon.tsxsrc/components/list-item/examples/list-item-interactive.tsxsrc/components/list-item/examples/list-item-radio.tsxsrc/components/list-item/list-item.tsxsrc/components/list-item/examples/list-item-icon-size.tsxsrc/components/list-item/menu-item-meta/menu-item-meta.tsxsrc/components/list-item/examples/list-item-command-text.tsxsrc/components/list-item/examples/list-item-pictures.tsxsrc/components/list-item/examples/list-item-primary-component.tsxsrc/components/list-item/examples/list-item-checkbox.tsxsrc/components/menu-list/menu-list.tsxsrc/components/list/list-renderer.tsxsrc/components/list-item/examples/list-item-basic.tsxsrc/components/list-item/examples/list-item-actions.tsxsrc/components/menu-list/menu-list-renderer.tsxsrc/components/menu/examples/menu-badge-icons.tsxsrc/components/list-item/examples/list-item-multiple-lines.tsx
⚙️ CodeRabbit configuration file
**/*.tsx: Our.tsxfiles are typically using StencilJS, not React. When a developer wants to return multiple top-level JSX elements from therendermethod, they will sometimes wrap them in an array literal. In these cases, rather than recommending they addkeyproperties to the elements, recommend removing the hardcoded array literal. Recommend wrapping the elements in StencilJS's special<Host>element.
Files:
src/components/list/list.tsxsrc/components/list-item/examples/list-item-icon.tsxsrc/components/list-item/examples/list-item-interactive.tsxsrc/components/list-item/examples/list-item-radio.tsxsrc/components/list-item/list-item.tsxsrc/components/list-item/examples/list-item-icon-size.tsxsrc/components/list-item/menu-item-meta/menu-item-meta.tsxsrc/components/list-item/examples/list-item-command-text.tsxsrc/components/list-item/examples/list-item-pictures.tsxsrc/components/list-item/examples/list-item-primary-component.tsxsrc/components/list-item/examples/list-item-checkbox.tsxsrc/components/menu-list/menu-list.tsxsrc/components/list/list-renderer.tsxsrc/components/list-item/examples/list-item-basic.tsxsrc/components/list-item/examples/list-item-actions.tsxsrc/components/menu-list/menu-list-renderer.tsxsrc/components/menu/examples/menu-badge-icons.tsxsrc/components/list-item/examples/list-item-multiple-lines.tsx
**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.
Files:
src/components/list/list.tsxsrc/components/menu-list/menu-list.e2e.tssrc/components/list-item/examples/list-item-icon.tsxsrc/components/list-item/examples/list-item-interactive.tsxsrc/components/list-item/examples/list-item-radio.tsxsrc/components/list-item/list-item.tsxsrc/components/list/list-item.types.tssrc/components/list/list.e2e.tssrc/components/list-item/examples/list-item-icon-size.tsxsrc/components/list-item/menu-item-meta/menu-item-meta.tsxsrc/components/list-item/examples/list-item-command-text.tsxsrc/components/list-item/examples/list-item-pictures.tsxsrc/components/list-item/examples/list-item-primary-component.tsxsrc/components/list-item/examples/list-item-checkbox.tsxsrc/components/menu-list/menu-list.tsxsrc/components/list/list-renderer.tsxsrc/components/list-item/examples/list-item-basic.tsxsrc/components/list-item/examples/list-item-actions.tsxsrc/components/menu-list/menu-list-renderer.tsxsrc/components/menu/examples/menu-badge-icons.tsxsrc/components/list-item/examples/list-item-multiple-lines.tsx
src/components/**/*.tsx
⚙️ CodeRabbit configuration file
src/components/**/*.tsx: When contributors add new props to existing components in the lime-elements repository, they should always add documentation examples that demonstrate the new prop's usage and explain how it works. This helps with user adoption, feature discoverability, and maintains comprehensive documentation.
Files:
src/components/list/list.tsxsrc/components/list-item/examples/list-item-icon.tsxsrc/components/list-item/examples/list-item-interactive.tsxsrc/components/list-item/examples/list-item-radio.tsxsrc/components/list-item/list-item.tsxsrc/components/list-item/examples/list-item-icon-size.tsxsrc/components/list-item/menu-item-meta/menu-item-meta.tsxsrc/components/list-item/examples/list-item-command-text.tsxsrc/components/list-item/examples/list-item-pictures.tsxsrc/components/list-item/examples/list-item-primary-component.tsxsrc/components/list-item/examples/list-item-checkbox.tsxsrc/components/menu-list/menu-list.tsxsrc/components/list/list-renderer.tsxsrc/components/list-item/examples/list-item-basic.tsxsrc/components/list-item/examples/list-item-actions.tsxsrc/components/menu-list/menu-list-renderer.tsxsrc/components/menu/examples/menu-badge-icons.tsxsrc/components/list-item/examples/list-item-multiple-lines.tsx
src/components/**/examples/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
src/components/**/examples/**/*.{ts,tsx}: These files are an exception to the rule that all imports should use relative paths. When these example files import something that is publicly exported by lime-elements, the import should be made from@limetech/lime-elements. If they import something from another file inside theexamplefolder, the import should use a relative path.
Files:
src/components/list-item/examples/list-item-icon.tsxsrc/components/list-item/examples/list-item-interactive.tsxsrc/components/list-item/examples/list-item-radio.tsxsrc/components/list-item/examples/list-item-icon-size.tsxsrc/components/list-item/examples/list-item-command-text.tsxsrc/components/list-item/examples/list-item-pictures.tsxsrc/components/list-item/examples/list-item-primary-component.tsxsrc/components/list-item/examples/list-item-checkbox.tsxsrc/components/list-item/examples/list-item-basic.tsxsrc/components/list-item/examples/list-item-actions.tsxsrc/components/menu/examples/menu-badge-icons.tsxsrc/components/list-item/examples/list-item-multiple-lines.tsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3518
File: src/components/notched-outline/examples/notched-outline-basic.scss:11-22
Timestamp: 2025-04-14T10:01:18.721Z
Learning: Examples in Lime Elements use Shadow DOM (with `shadow: true` in the component decorator) for style encapsulation, making additional class-based selectors unnecessary. This ensures styles are automatically scoped to each component without leaking to the rest of the application.
📚 Learning: 2025-04-29T14:15:32.104Z
Learnt from: adrianschmidt
PR: Lundalogik/lime-elements#3529
File: src/components/ai-avatar/ai-avatar.scss:20-22
Timestamp: 2025-04-29T14:15:32.104Z
Learning: In components using Shadow DOM (like those in lime-elements), the universal selector `*` is already scoped to the component's shadow tree and won't cause unwanted side effects outside the component. There's no functional difference between `*` and `:host *` in this context.
Applied to files:
src/components/list-item/examples/list-item-interactive.scsssrc/components/list-item/examples/list-item-basic.scsssrc/components/menu-list/menu-list.scsssrc/components/list-item/list-item.scss
📚 Learning: 2025-04-14T10:01:18.721Z
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3518
File: src/components/notched-outline/examples/notched-outline-basic.scss:11-22
Timestamp: 2025-04-14T10:01:18.721Z
Learning: Examples in Lime Elements use Shadow DOM (with `shadow: true` in the component decorator) for style encapsulation, making additional class-based selectors unnecessary. This ensures styles are automatically scoped to each component without leaking to the rest of the application.
Applied to files:
src/components/list-item/examples/list-item-interactive.scsssrc/components/list-item/list-item.scss
📚 Learning: 2025-08-12T14:01:27.846Z
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3638
File: src/components/radio-button-group/radio-button-group.tsx:91-91
Timestamp: 2025-08-12T14:01:27.846Z
Learning: The `limel-radio-button-group` component has accessibility concerns because it uses `limel-list` for rendering, which doesn't follow WAI-ARIA APG keyboard navigation patterns for radio button groups. Arrow keys should move focus AND select in radio groups, but limel-list requires arrow keys to move focus, then Enter/Space to select.
Applied to files:
src/components/list/list.tsxsrc/components/list-item/examples/list-item-radio.tsxsrc/components/list-item/list-item.scss
📚 Learning: 2025-07-30T14:20:41.791Z
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3624
File: src/components/button-group/button-group.scss:115-118
Timestamp: 2025-07-30T14:20:41.791Z
Learning: In lime-elements, CSS custom properties like `--limel-theme-surface-background-color` are applied at :root level via `core-styles.scss` and should be expected to be available without fallbacks. The library's architecture guarantees these theme variables are present, so adding fallbacks would be redundant and go against the design intention.
Applied to files:
src/components/list/examples/list-badge-icons.scsssrc/components/list-item/list-item.scss
📚 Learning: 2025-08-12T14:01:27.846Z
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3638
File: src/components/radio-button-group/radio-button-group.tsx:91-91
Timestamp: 2025-08-12T14:01:27.846Z
Learning: The `limel-radio-button-group` component allows deselection of radio buttons (unlike native radio buttons), which improves UX by letting users leave questions unanswered, but diverges from standard radio button behavior expectations.
Applied to files:
src/components/list-item/examples/list-item-radio.tsxsrc/components/list-item/list-item.scss
📚 Learning: 2025-08-07T14:39:00.053Z
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3638
File: src/components/radio-button-group/radio-button-group.tsx:32-32
Timestamp: 2025-08-07T14:39:00.053Z
Learning: The `limel-radio-button-group` component uses `shadow: false` because it's a pure wrapper around `limel-list` with no component-specific styles, following the pattern of other wrapper components like `limel-action-bar-overflow-menu`.
Applied to files:
src/components/list-item/examples/list-item-radio.tsxsrc/components/menu-list/menu-list.scsssrc/components/list-item/list-item.scsssrc/components/list/list.scss
📚 Learning: 2025-04-25T14:22:02.157Z
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3529
File: src/components/lime-ai-avatar/lime-ai-avatar.scss:24-28
Timestamp: 2025-04-25T14:22:02.157Z
Learning: In lime-elements, CSS custom properties follow a specific naming convention:
1. Internal use variables (not for consumers) are prefixed with `--limel-component-name-`
2. External use variables (intended for consumers) are prefixed with `--component-name-`
Variables documented with JSDoc comments are typically meant for external use.
Applied to files:
src/components/list-item/list-item.scss
📚 Learning: 2025-04-16T14:14:18.253Z
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3530
File: src/components/text-editor/examples/text-editor-composite.tsx:4-7
Timestamp: 2025-04-16T14:14:18.253Z
Learning: For lime-elements, example files should be self-contained and avoid importing internal implementation details. Example files should only import public exports from 'limetech/lime-elements' or use relative imports for other files within the examples folder. Duplicating simple type definitions in example files is preferred over importing internal types.
Applied to files:
src/components/list-item/examples/list-item-checkbox.tsxsrc/components/list-item/examples/list-item-basic.tsx
🧬 Code graph analysis (13)
src/components/list-item/examples/list-item-interactive.tsx (3)
src/components/list-item/list-item.tsx (1)
Component(37-400)src/components/list-item/examples/list-item-actions.tsx (1)
Component(18-89)src/components/list-item/examples/list-item-checkbox.tsx (1)
Component(27-138)
src/components/list-item/examples/list-item-radio.tsx (2)
src/components/list-item/list-item.tsx (1)
Component(37-400)src/components/list-item/examples/list-item-checkbox.tsx (1)
Component(27-138)
src/components/list-item/list-item.tsx (7)
src/components/list/list-item.types.ts (2)
ListItem(11-72)ListSeparator(90-90)src/components/icon/icon.types.ts (1)
IconSize(4-4)src/util/random-string.ts (1)
createRandomString(1-10)src/components/icon/get-icon-props.ts (1)
getIconName(11-24)src/components/menu/menu.types.ts (1)
MenuItem(58-135)src/components/radio-button-group/radio-button.template.tsx (1)
RadioButtonTemplate(39-64)src/components/checkbox/checkbox.template.tsx (1)
CheckboxTemplate(20-102)
src/components/list-item/examples/list-item-icon-size.tsx (3)
src/components/list-item/list-item.tsx (1)
Component(37-400)src/components/icon/icon.types.ts (1)
IconSize(4-4)src/components/select/option.types.ts (1)
Option(7-56)
src/components/list-item/menu-item-meta/menu-item-meta.tsx (2)
src/components/list-item/examples/list-item-command-text.tsx (1)
Component(14-60)src/components/list-item/list-item.tsx (1)
Component(37-400)
src/components/list-item/examples/list-item-command-text.tsx (2)
src/components/list-item/list-item.tsx (1)
Component(37-400)src/components/list-item/menu-item-meta/menu-item-meta.tsx (1)
Component(13-70)
src/components/list-item/examples/list-item-pictures.tsx (2)
src/components/list-item/list-item.tsx (1)
Component(37-400)src/components/list-item/examples/list-item-icon.tsx (1)
Component(10-64)
src/components/list-item/examples/list-item-primary-component.tsx (2)
src/components/list-item/list-item.tsx (1)
Component(37-400)src/components/menu/menu.types.ts (1)
MenuItem(58-135)
src/components/list-item/examples/list-item-checkbox.tsx (3)
src/components/list-item/list-item.tsx (1)
Component(37-400)src/components/list-item/examples/list-item-radio.tsx (1)
Component(27-115)src/components/list/list-item.types.ts (1)
ListItem(11-72)
src/components/list-item/examples/list-item-basic.tsx (1)
src/components/list-item/list-item.tsx (1)
Component(37-400)
src/components/list-item/examples/list-item-actions.tsx (4)
src/components/list-item/list-item.tsx (1)
Component(37-400)src/components/list-item/examples/list-item-checkbox.tsx (1)
Component(27-138)src/components/menu/menu.types.ts (1)
MenuItem(58-135)src/components/list/list-item.types.ts (2)
ListSeparator(90-90)ListItem(11-72)
src/components/menu-list/menu-list-renderer.tsx (1)
src/components/list/list-item.types.ts (1)
ListSeparator(90-90)
src/components/list-item/examples/list-item-multiple-lines.tsx (2)
src/components/list-item/list-item.tsx (1)
Component(37-400)src/components/select/option.types.ts (1)
Option(7-56)
🪛 Biome (2.1.2)
src/components/list-item/examples/list-item-interactive.tsx
[error] 41-50: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 51-51: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 63-63: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/components/list-item/list-item.tsx
[error] 177-178: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
src/components/list-item/examples/list-item-checkbox.tsx
[error] 76-76: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
src/components/list/list-renderer.tsx
[error] 42-46: The ARIA attribute 'aria-orientation' is not supported by this element.
Ensure that ARIA attributes are valid for the role of the element.
(lint/a11y/useAriaPropsSupportedByRole)
🔇 Additional comments (27)
src/components/list/examples/list-badge-icons.scss (1)
4-4: Token switch to glaucous-dark looks consistent with our palette.No functional concerns from me. Please confirm visual contrast with common icon glyphs in both light/dark themes during visual QA.
src/components/list/partial-styles/_has-grid-layout.scss (1)
2-2: Grid gap reduced to 0.5rem — check dense layouts and hover/focus states.Looks good with the PR-wide radius/spacing updates. Please verify that:
- Focus ring and selection backgrounds don’t visually collide at tighter gaps.
- Two- and three-column breakpoints still read well for long secondary text.
src/components/menu/examples/menu-badge-icons.tsx (1)
17-41: LGTM on token updates and separator additionThe color token changes and the new Delete entry align with the Icon interface and badge icon styling. No API issues spotted.
src/components/list-item/examples/list-item-radio.tsx (2)
57-74: Consider adding arrow-key selection to match WAI-ARIA Radio Group APGCurrent behavior requires Enter/Space to select; arrow keys only move focus. If you want to match APG, handle ArrowLeft/Right/Up/Down on the radiogroup container to update
selectedValueand call.focus()on the newly selectedlimel-list-item.Do you want me to add a minimal arrow-key handler example here?
1-1: Use a type-only import to avoid bundling typesImport the ListItem type with
import typeto ensure it’s erased at compile time and not included in runtime bundles.-import { ListItem } from '@limetech/lime-elements'; +import type { ListItem } from '@limetech/lime-elements';⛔ Skipped due to learnings
Learnt from: adrianschmidt PR: Lundalogik/lime-elements#3530 File: src/components/text-editor/examples/text-editor-composite.tsx:4-7 Timestamp: 2025-04-17T09:39:36.254Z Learning: For lime-elements, example files should import types from the public API using 'limetech/lime-elements' rather than defining duplicate types locally. This includes types like `EditorUiType` which are part of the component's public API.Learnt from: adrianschmidt PR: Lundalogik/lime-elements#3464 File: src/components/text-editor/prosemirror-adapter/plugins/image/inserter.ts:1-11 Timestamp: 2025-03-04T13:48:44.712Z Learning: The lime-elements codebase must use relative imports (even with multiple "../") rather than alias imports or re-exports because they export types to `<repo-root>/dist/types`. Using non-relative imports breaks type generation as import paths from generated type files point to full .ts files rather than .d.ts files.Learnt from: Kiarokh PR: Lundalogik/lime-elements#3530 File: src/components/text-editor/examples/text-editor-composite.tsx:4-7 Timestamp: 2025-04-16T14:14:18.253Z Learning: For lime-elements, example files should be self-contained and avoid importing internal implementation details. This includes not importing types from internal files like '../types.ts', especially those marked with beta. Duplicating simple type definitions in example files is preferred over importing internal types that may change.Learnt from: adrianschmidt PR: Lundalogik/lime-elements#3464 File: src/components/text-editor/prosemirror-adapter/plugins/image/inserter.ts:1-11 Timestamp: 2025-03-04T14:27:29.714Z Learning: In example components (typically found in `src/components/*/examples/`), imports of lime-elements exports should use `limetech/lime-elements` instead of relative paths. This is because example components aren't included in type declarations and are used in documentation to show developers how to properly use lime-elements components.Learnt from: Kiarokh PR: Lundalogik/lime-elements#3530 File: src/components/text-editor/examples/text-editor-composite.tsx:4-7 Timestamp: 2025-04-16T14:14:18.253Z Learning: For lime-elements, example files should be self-contained and avoid importing internal implementation details. Example files should only import public exports from 'limetech/lime-elements' or use relative imports for other files within the examples folder. Duplicating simple type definitions in example files is preferred over importing internal types.src/components/menu-list/menu-list.tsx (1)
146-162: Double emission ofselectevent inlimel-menu-list
ThehandleSingleSelectmethod (src/components/menu-list/menu-list.tsx, lines 136–165) currently emits theselectevent twice: once to “deselect” the previously selected item, and then again for the newly clicked item. However, the@Event()JSDoc forselectonly describes “a new value [being] selected.”Please confirm which API is intended:
- If you want to notify both deselection and selection, consider splitting these into two distinct events (e.g.
deselectandselect) or updating the documentation and event name to reflect a dual-emission API.- If only the activation of the clicked item should be announced, remove the first emission so that
selectfires exactly once per interaction.After confirming, update the implementation, add or adjust tests accordingly, and clarify the public API in the documentation.
src/components/list-item/examples/list-item-icon.tsx (2)
19-58: Solid example structure and Stencil patterns are correct
- Uses
<Host>for multiple top-level nodes per our Stencil guideline.- Local state + typed handler (
CustomEvent<boolean>) is correct;stopPropagation()matches our example conventions.- Props passed to
limel-list-itemalign with the component API (iconobject,badgeIcon,secondaryText).
23-34: Consider aligning focus/semantics: keep tabindex for demo, but also set type to “menuitem”These items are focusable (tabindex="0") but are not selectable. Setting
type="menuitem"improves ARIA semantics without changing behavior. Keeptabindex="0"so keyboard users can still focus items in this example.Please confirm this example is intended to be focusable. If not, drop the
tabindexattributes instead.<limel-list-item value={1} + type="menuitem" tabindex="0" text="Santa Hat" secondaryText="Santa's favorite" icon={{ name: 'santas_hat', title: 'Icon of Santa Hat', color: 'rgb(var(--color-coral-default))', }} badgeIcon={this.badgeIcon} /> <limel-list-item value={2} + type="menuitem" tabindex="0" text="Party Hat" secondaryText="For the party animals" icon={{ name: 'party_hat', title: 'Icon of Party Hat', color: 'rgb(var(--color-white))', backgroundColor: 'rgb(var(--color-pink-default))', }} badgeIcon={this.badgeIcon} />Also applies to: 35-47
⛔ Skipped due to learnings
Learnt from: Kiarokh PR: Lundalogik/lime-elements#3638 File: src/components/radio-button-group/radio-button-group.tsx:91-91 Timestamp: 2025-08-12T14:01:27.846Z Learning: The `limel-radio-button-group` component has accessibility concerns because it uses `limel-list` for rendering, which doesn't follow WAI-ARIA APG keyboard navigation patterns for radio button groups. Arrow keys should move focus AND select in radio groups, but limel-list requires arrow keys to move focus, then Enter/Space to select.src/components/list/partial-styles/custom-styles.scss (1)
27-31: Hover/focus stacking context: verify no regression after removing z-index logicPrevious MDC styles often bumped z-index on hover to avoid box-shadow/outline clipping. We now only apply
is-flat-clickable()with no z-index change. Please verify focus ring/hover visuals aren’t obscured by adjacent items or containers.src/components/list-item/examples/list-item-multiple-lines.tsx (1)
1-3: Imports follow our examples guidelineImporting
Optionfrom@limetech/lime-elementsis correct for files undersrc/components/**/examples/**.src/components/list-item/examples/list-item-basic.scss (1)
34-57: Good: shadow-DOM scoped selectors, no BEM, and consistent interaction visuals
- Selectors target
limel-list-itemdirectly and avoid BEM as per our guideline.- Uses our shared mixins for clickable + keyboard focus.
- Corner rounding on first/last items is a nice touch for demos.
src/components/list-item/examples/list-item-command-text.tsx (1)
20-58: Clear, focused example; primaryComponent usage is correct
- Demonstrates
limel-menu-item-metaviaprimaryComponentas intended.- Single root element (
<ul>) is fine per our Stencil rule.src/components/list-item/examples/list-item-pictures.tsx (1)
56-59: LGTM: state handling for the control is correctStops propagation and updates state from
event.detailas expected.src/components/list-item/examples/list-item-basic.tsx (1)
29-50: LGTM: minimal, well-documented example aligned with component semanticsSingle
<ul>root is fine (no need for<Host>), and the note on focusability and consumer-applied mixins matches our guidance.src/components/list-item/menu-item-meta/menu-item-meta.tsx (1)
11-12: Confirm public API exposure oflimel-menu-item-metaI didn’t find any re-exports of the
limel-menu-item-metacomponent insrc/index.tsor any barrel files, so it isn’t part of the TypeScript public API. However, by default Stencil will still include every component in its loader bundle—JSDoc’s@privateonly hides it from generated docs, not from consumers’ runtime.Please verify the following before merging:
- The published loader (e.g. in
dist/loader) does not register thelimel-menu-item-metatag.- There’s no reference to this component in the package’s published entrypoints (
main,module, orexportsinpackage.json).- If the intent is to fully hide it, consider configuring Stencil to exclude it (for example via an
internalflag instencil.config.tsor moving it out of the scanned components directory).src/components/list-item/examples/list-item-primary-component.tsx (1)
26-32: LGTM after type fix: action items demonstrate the actions menu correctlyGood coverage of enabled/disabled actions and separators.
src/components/list/list.e2e.ts (4)
45-46: LGTM: Assert item content via reflected attributes.Asserting
textvia reflected attributes on<limel-list-item>is robust and avoids DOM structure coupling.
73-74: LGTM: Disabled item text assertion via attributes.
183-192: LGTM: Selectable semantics verified on items (role/type="option").This aligns with the new renderer mapping
selectable→option.
205-205: LGTM: Interact withlimel-list-itemdirectly.Querying the component rather than internal DOM reduces brittleness of the test.
src/components/list/list-renderer.tsx (2)
116-123: LGTM: Clear, explicit mapping from list config to itemtype.The mapping maintains semantics while decoupling the renderer from MDC internals.
126-141: LGTM: Unified item rendering through<limel-list-item>.Passing reflected attributes +
data-indexpreserves testability and reduces DOM coupling.src/components/list/partial-styles/_static-actions.scss (1)
21-31: Spacing tweaks: check for layout shifts with sticky headers/footersChanging margins/padding to fixed rem values affects the scroll snapping experience and can shift content when the sticky header/footers toggle classes. Validate in long lists with both top and bottom static actions to ensure no jump or overlap occurs at the transition.
src/components/list/list.scss (3)
3-3: Border radius change: verify across list variants and themesIncreasing
$list-border-radiusto0.5remaffects list-group visuals (grid layout, striped rows, static actions, nested lists). Confirm there are no visual regressions, especially where items abut container edges.
64-95: Interactive states: good focus visualization; consider motion preferences and stackingThe focus/hover/active rules and
visualize-keyboard-focus()usage are solid. Since you bumpz-indexto 1, ensure it doesn't conflict with sticky headers/footers or menus that also use z-index. You may also want to disable transitions underprefers-reduced-motion.
40-63: Divider primitives: API looks cleaner than MDC—niceThe new
.limel-list-divider-*primitives improve clarity and control. Good use of truncation and token colors.src/components/list-item/list-item.tsx (1)
24-35: Documentation examples foriconSize,badgeIcon, andtypealready existNo further additions are needed—the existing example files cover the new props:
- iconSize: demonstrated in src/components/list-item/examples/list-item-icon-size.tsx
- badgeIcon: used in list-item-icon.tsx, list-item-pictures.tsx, list-item-radio.tsx, and list-item-checkbox.tsx
- type: shown in list-item-radio.tsx, list-item-interactive.tsx, list-item-checkbox.tsx, and list-item-actions.tsx
Likely an incorrect or invalid review comment.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Kiarokh
force-pushed
the
list-item
branch
2 times, most recently
from
97c4258 to
06ea394
Compare
| limel-list-item:not([disabled]):not([disabled='true']):hover, | ||
| limel-list-item:not([disabled]):not([disabled='true']):focus-visible, | ||
| limel-list-item:not([disabled]):not([disabled='true']):focus-within { | ||
| background-color: rgb(var(--contrast-400)); |
There was a problem hiding this comment.
When the first item is selected and another item is active it looks confusing
Kiarokh
force-pushed
the
list-item
branch
3 times, most recently
from
10f3f3a to
c5f885b
Compare
|
Tested both light and dark mode. Also linked to webclient. Looks good! 🚀 Great Job Kia! ❤️ |
This internal component will make the implementation of the new `limel-list-item` easier in the `menu-list-renderer`.
renamed `rendertext` to `renderTextForSeparator` for clearance and consistency with the `list-renderer`
Use `limel-list-item` to render the menu items. Note: if the deprecated `iconColor` is defined as a prop for menu items, it will have no visible visual effect any longer. The `iconColor` prop however, is not removed yet. Therefore your builds will not fail! But you are highly encouraged to use the `Icon` interface instead, to set the `color` or `backgroundColor` of the icons, as well described in the documentations. The deprecated `iconColor` property will be removed soon.
The maximum number of lines of `secondaryText` has never been decided by the consumer. It has been defined as an inline style in the `menu-list-renderer`.
Use `limel-list-item` to render the list items. Note: if the deprecated `iconColor` is defined as a prop for list items, it will have no visible visual effect any longer. The `iconColor` prop however, is not removed yet. Therefore your builds will not fail! But you are highly encouraged to use the `Icon` interface instead, to set the `color` or `backgroundColor` of the icons, as well described in the documentations. The deprecated `iconColor` property will be removed soon.
The `aria-orientation` attribute is simply not valid for `role="group"` (which is used for checkboxes), according to the ARIA spec. This is regardless of how the keyboard navigation actually works.
Using `aria-haspopup="menu"` + `aria-expanded` we properly inform assistive tech that a menuitem has nested items.
…item` folder Updated imports in: list-item.tsx, list-renderer.tsx, list.tsx, interface.ts, select.tsx, select.template.tsx, menu.tsx, menu-list-renderer.tsx, menu-list.tsx, input-field.tsx, file-viewer.tsx, progress-flow.types.ts, radio-button-group.tsx, picker.tsx, and a few examples.
|
🎉 This PR is included in version 38.24.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fix https://github.com/Lundalogik/crm-client/issues/216
Summary by CodeRabbit
MDC dependency
Completely removed the dependency on MDC's styles for list and list items. Now we own the CSS entirely, and only rely on MDC's foundation for a few details such as handling some interactions with list items, and dynamic switching of tabindex on items.
How to test
List items are everywhere; they are in menus, select-dropdowns, input field suggestions, list, radio button group, etc… Check all these to make sure they work like before.
Review:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: