-
Notifications
You must be signed in to change notification settings - Fork 16
docs(list): improve examples & clarify things a bit more #3651
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
Conversation
|
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 📝 WalkthroughWalkthroughUpdates multiple list example components: add reactive state and UI controls (icon toggles, badge icon toggle), display last event/action, adjust example data and docs, and wrap some renders in Host. Two example components (checkbox and radio-button) are removed, and list.tsx JSDoc example references are reordered. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant L as limel-list
participant EX as Example Component
participant V as limel-example-value
U->>L: select action (onSelect)
L-->>EX: LimelListCustomEvent<ListItem>
rect rgba(214,234,248,0.3)
note right of EX: Update reactive state
EX->>EX: onSelectAction formats "Executing action: ..."
EX->>EX: this.lastEvent = message
end
EX-->>V: value=lastEvent (re-render)
sequenceDiagram
participant U as User
participant C as limel-checkbox
participant EX as Example Component
participant L as limel-list
U->>C: toggle "icon"/"badgeIcons"
C-->>EX: CustomEvent<boolean>
rect rgba(232,248,245,0.4)
note right of EX: Sync state and items
EX->>EX: stopPropagation()
EX->>EX: update state (showIcons/badgeIcons)
EX->>EX: map items to set/restore icons (if applicable)
end
EX-->>L: props updated (items, badgeIcons)
sequenceDiagram
participant U as User
participant L as limel-list (selectable)
participant EX as Example Component
participant V as limel-example-value
U->>L: change selection
L-->>EX: LimelListCustomEvent<ListItem>
EX->>EX: lastEvent = "Selected: {text}, selected: {selected}"
EX->>EX: update items with event.detail
EX-->>V: value=lastEvent
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3651/ |
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.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/list/examples/list-radio-button-icons.tsx (1)
96-111: Don’t return an array of JSX elements; wrap in Host.Per our Stencil guidelines, avoid array literals at the top level. This also sidesteps “missing key” lints.
Apply this diff:
- public render() { - return [ - <limel-list - onChange={this.handleChange} - items={this.items} - type="radio" - />, - <limel-example-value value={this.selectedItem} />, - <limel-example-controls> - <limel-checkbox - checked={this.showIcons} - label="icon" - onChange={this.setIcon} - /> - </limel-example-controls>, - ]; - } + public render() { + return ( + <Host> + <limel-list + onChange={this.handleChange} + items={this.items} + type="radio" + /> + <limel-example-value value={this.selectedItem} /> + <limel-example-controls> + <limel-checkbox + checked={this.showIcons} + label="icon" + onChange={this.setIcon} + /> + </limel-example-controls> + </Host> + ); + }Additionally, ensure
Hostis imported:// at top of file import { Component, h, State, Host } from '@stencil/core';src/components/list/examples/list-checkbox-icons.tsx (2)
96-112: Wrap multiple top-level elements in Host instead of an arrayStencil examples must not return an array. Wrap siblings in to satisfy our guidelines and fix the Biome “Missing key property in iterable” error.
Apply this diff:
- public render() { - return [ - <limel-list - onChange={this.handleChange} - items={this.items} - type="checkbox" - />, - <limel-example-value value={this.selectedItems} />, - <limel-example-controls> - <limel-checkbox - checked={this.showIcons} - label="icon" - onChange={this.setIcon} - /> - </limel-example-controls>, - ]; - } + public render() { + return ( + <Host> + <limel-list + onChange={this.handleChange} + items={this.items} + type="checkbox" + /> + <limel-example-value value={this.selectedItems} /> + <limel-example-controls> + <limel-checkbox + checked={this.showIcons} + label="icon" + onChange={this.setIcon} + /> + </limel-example-controls> + </Host> + ); + }Remember to import Host (see separate comment).
2-2: Import Host from @stencil/coreRequired to use the Host wrapper in render.
-import { Component, h, State } from '@stencil/core'; +import { Component, h, Host, State } from '@stencil/core';
📜 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 (9)
src/components/list/examples/list-action.tsx(3 hunks)src/components/list/examples/list-badge-icons.tsx(3 hunks)src/components/list/examples/list-checkbox-icons.tsx(4 hunks)src/components/list/examples/list-checkbox.tsx(0 hunks)src/components/list/examples/list-primary-component.tsx(5 hunks)src/components/list/examples/list-radio-button-icons.tsx(4 hunks)src/components/list/examples/list-radio-button.tsx(0 hunks)src/components/list/examples/list-selectable.tsx(2 hunks)src/components/list/list.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- src/components/list/examples/list-checkbox.tsx
- src/components/list/examples/list-radio-button.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/examples/list-primary-component.tsxsrc/components/list/examples/list-badge-icons.tsxsrc/components/list/examples/list-selectable.tsxsrc/components/list/examples/list-radio-button-icons.tsxsrc/components/list/examples/list-checkbox-icons.tsxsrc/components/list/examples/list-action.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/examples/list-primary-component.tsxsrc/components/list/examples/list-badge-icons.tsxsrc/components/list/examples/list-selectable.tsxsrc/components/list/examples/list-radio-button-icons.tsxsrc/components/list/examples/list-checkbox-icons.tsxsrc/components/list/examples/list-action.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/list/examples/list-primary-component.tsxsrc/components/list/examples/list-badge-icons.tsxsrc/components/list/examples/list-selectable.tsxsrc/components/list/examples/list-radio-button-icons.tsxsrc/components/list/examples/list-checkbox-icons.tsxsrc/components/list/examples/list-action.tsx
**/*.{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/list.tsxsrc/components/list/examples/list-primary-component.tsxsrc/components/list/examples/list-badge-icons.tsxsrc/components/list/examples/list-selectable.tsxsrc/components/list/examples/list-radio-button-icons.tsxsrc/components/list/examples/list-checkbox-icons.tsxsrc/components/list/examples/list-action.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/examples/list-primary-component.tsxsrc/components/list/examples/list-badge-icons.tsxsrc/components/list/examples/list-selectable.tsxsrc/components/list/examples/list-radio-button-icons.tsxsrc/components/list/examples/list-checkbox-icons.tsxsrc/components/list/examples/list-action.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/examples/list-primary-component.tsxsrc/components/list/examples/list-badge-icons.tsxsrc/components/list/examples/list-selectable.tsxsrc/components/list/examples/list-radio-button-icons.tsxsrc/components/list/examples/list-checkbox-icons.tsxsrc/components/list/examples/list-action.tsx
🧠 Learnings (7)
📓 Common learnings
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3581
File: src/components/chip-set/chip-set.tsx:626-626
Timestamp: 2025-07-02T12:49:21.522Z
Learning: 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.
📚 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/list.tsx
📚 Learning: 2025-03-04T14:27:29.714Z
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.
Applied to files:
src/components/list/list.tsx
📚 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/list.tsxsrc/components/list/examples/list-radio-button-icons.tsx
📚 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/list.tsxsrc/components/list/examples/list-radio-button-icons.tsxsrc/components/list/examples/list-checkbox-icons.tsx
📚 Learning: 2025-04-25T13:32:56.396Z
Learnt from: adrianschmidt
PR: Lundalogik/lime-elements#3529
File: src/components/lime-ai-avatar/lime-ai-avatar.tsx:30-38
Timestamp: 2025-04-25T13:32:56.396Z
Learning: For StencilJS components in lime-elements, prefer using the `<Host>` element to wrap child elements in the render method instead of returning an array of elements, which eliminates the need for key attributes and follows StencilJS best practices.
Applied to files:
src/components/list/examples/list-badge-icons.tsx
📚 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/examples/list-radio-button-icons.tsx
🧬 Code Graph Analysis (5)
src/components/list/examples/list-badge-icons.tsx (4)
src/components/list/examples/list-radio-button-icons.tsx (1)
Component(12-135)src/components/list/list.tsx (1)
Component(37-302)src/components/list/examples/list-checkbox-icons.tsx (1)
Component(12-135)src/components/list/examples/list-grid.tsx (1)
Component(15-69)
src/components/list/examples/list-selectable.tsx (1)
src/components/list/list-item.types.ts (1)
ListItem(11-71)
src/components/list/examples/list-radio-button-icons.tsx (1)
src/components/list/list-item.types.ts (1)
ListItem(11-71)
src/components/list/examples/list-checkbox-icons.tsx (1)
src/components/list/list-item.types.ts (1)
ListItem(11-71)
src/components/list/examples/list-action.tsx (1)
src/components/list/list-item.types.ts (1)
ListItem(11-71)
🪛 Biome (2.1.2)
src/components/list/examples/list-radio-button-icons.tsx
[error] 104-104: 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/examples/list-checkbox-icons.tsx
[error] 104-104: 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/examples/list-action.tsx
[error] 32-32: 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] 33-33: 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)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Docs / Publish Docs
- GitHub Check: Test
- GitHub Check: Build
🔇 Additional comments (10)
src/components/list/list.tsx (1)
28-28: Docs update aligns with new examples (good).Replacing the removed checkbox/radio examples with
limel-example-list-selectablehere is consistent with the new example set introduced in this PR.src/components/list/examples/list-primary-component.tsx (1)
39-43: Good example of ordering the primary component.Explicitly demonstrating
style: { order: 3 }together withsize: 'small'clearly shows how placement and sizing can be customized.src/components/list/examples/list-selectable.tsx (3)
41-42: Nice touch adding a disabled item to illustrate behavior.Demonstrates disabled-state interaction clearly for documentation readers.
45-47: Useful reactive “last event” display.Good addition to make the example interactive and self-explanatory.
50-60: Correct use of Host when returning multiple elements.Wrapping in
<Host>adheres to our Stencil pattern and avoids array returns.src/components/list/examples/list-badge-icons.tsx (2)
66-78: Good interactive example and correct Host usage.Binding
badgeIconsto UI controls makes the behavior tangible and follows our examples’ pattern.
81-84: Event handling is correct.Stopping propagation on the example control avoids unintended side effects.
src/components/list/examples/list-radio-button-icons.tsx (2)
7-10: Radio-list docs addition is clear and accurate.Nicely explains the single-selection behavior when
type="radio".
125-134: Icon toggle logic is sound.Restoring from a cached
originalIconsmap avoids mutating source data and keeps the example predictable.src/components/list/examples/list-checkbox-icons.tsx (1)
80-84: Icon toggling and caching approach looks goodGood use of local cache keyed by value and immutably updating items. This keeps example logic simple and predictable.
Also applies to: 89-93
| private onSelectAction = (event: LimelListCustomEvent<ListItem>) => { | ||
| const detail = event.detail as any; | ||
| const valuePart = | ||
| detail && 'value' in detail ? `, value: ${detail.value}` : ''; | ||
| this.lastEvent = `Executing action: ${detail?.text ?? 'unknown'}${valuePart}`; | ||
| }; |
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.
🧹 Nitpick (assertive)
Narrow the event/detail types and drop the any-cast
Use the generic for the list item value type and rely on optional chaining for value/text.
- private onSelectAction = (event: LimelListCustomEvent<ListItem>) => {
- const detail = event.detail as any;
- const valuePart =
- detail && 'value' in detail ? `, value: ${detail.value}` : '';
- this.lastEvent = `Executing action: ${detail?.text ?? 'unknown'}${valuePart}`;
- };
+ private onSelectAction = (
+ event: LimelListCustomEvent<ListItem<number>>,
+ ) => {
+ const detail = event.detail;
+ const valuePart =
+ typeof detail?.value !== 'undefined' ? `, value: ${detail.value}` : '';
+ this.lastEvent = `Executing action: ${detail?.text ?? 'unknown'}${valuePart}`;
+ };📝 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.
| private onSelectAction = (event: LimelListCustomEvent<ListItem>) => { | |
| const detail = event.detail as any; | |
| const valuePart = | |
| detail && 'value' in detail ? `, value: ${detail.value}` : ''; | |
| this.lastEvent = `Executing action: ${detail?.text ?? 'unknown'}${valuePart}`; | |
| }; | |
| private onSelectAction = ( | |
| event: LimelListCustomEvent<ListItem<number>>, | |
| ) => { | |
| const detail = event.detail; | |
| const valuePart = | |
| typeof detail?.value !== 'undefined' ? `, value: ${detail.value}` : ''; | |
| this.lastEvent = `Executing action: ${detail?.text ?? 'unknown'}${valuePart}`; | |
| }; |
🤖 Prompt for AI Agents
In src/components/list/examples/list-action.tsx around lines 37 to 42, the
handler currently casts event.detail to any; replace that with the proper
LimelListCustomEvent<ListItem<T>> generic (or the concrete
ListItem<string|number|...> type used here) so event is strongly typed, remove
the any-cast, and read detail using optional chaining (e.g. event.detail?.text
and event.detail?.value) when building the lastEvent string; ensure the handler
signature uses the generic event type and update value/text checks to rely on
optional chaining instead of the 'in' operator.
c1eb752 to
8dcc0ad
Compare
This will also make it easier to test the implementation of the new `limel-list-item` component later.
c5da3f6 to
4303cef
Compare
|
🎉 This PR is included in version 38.22.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This will also make it easier to test the implementation of the new
limel-list-itemcomponent later.related to https://github.com/Lundalogik/crm-client/issues/216
Summary by CodeRabbit
New Features
Documentation
Chores
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: