fix(list): prevent double `interact` event emits, when an item is clicked #3706

Kiarokh · 2025-10-15T09:12:45Z

fix https://github.com/Lundalogik/crm-client/issues/367
fix https://github.com/Lundalogik/crm-client/issues/364

Summary by CodeRabbit

New Features
- Improved keyboard support: Enter/Space now toggle selection on focused list items.
- Centralized interaction handling for more consistent selection across list variants.
Bug Fixes
- Clicking action menus no longer toggles items.
- Disabled and aria-disabled items correctly ignore clicks and key presses.
- More accurate selection state and feedback messages (Selected/Deselected).
Refactor
- Moved interaction handling from individual items to the list container for consistent behavior.

Review:

Commits are atomic
Commits have the correct type for the changes made
Commits with breaking changes are marked as such

Browsers tested:

(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)

Windows:

Chrome
Edge
Firefox

Linux:

Chrome
Firefox

macOS:

Chrome
Firefox
Safari

Mobile:

Chrome on Android
iOS

coderabbitai · 2025-10-15T09:13:13Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

Centralizes interaction handling in list-item examples to container-level click/keyboard events and removes the list-item component’s internal click handling and interact event. Selection state and “last interaction” messages are now managed by the example containers using data-value attributes. No public API signatures remain for the removed interact event.

Changes

Cohort / File(s)	Summary of changes
Core list-item interaction removal `src/components/list-item/list-item.tsx`	Removed internal click-based selection logic and the interact EventEmitter; dropped isSelectableType and related handlers; keyboard now triggers host click without internal toggling; ARIA/role handling retained.
Examples: centralized container handlers `src/components/list-item/examples/list-item-actions.tsx`, `src/components/list-item/examples/list-item-checkbox.tsx`, `src/components/list-item/examples/list-item-interactive.tsx`, `src/components/list-item/examples/list-item-radio.tsx`	Replaced per-item onInteract with ul-level onClick/onKeyDown; added data-value on items; introduced toggle/select helpers; guard against disabled and action menu targets; maintain selected state and last interaction strings; removed onInteract wiring.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant UL as List Container (ul)
  participant LI as <limel-list-item>
  participant State as Example State

  U->>LI: Click or Key (Enter/Space)
  note right of LI: No internal interact event<br/>No internal selection toggle
  LI-->>UL: Event bubbles (click/keydown)
  UL->>UL: Guard checks (disabled, action menu, aria-disabled)
  UL->>State: Toggle/update selected value(s)
  State-->>UL: Update lastInteraction text
  UL-->>U: Rerender with updated selection

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Add limel-list-item & menu-item-meta private components / refactor & improve list renderers / remove dependency to MDC's styles for list & list-items #3628 — Also removes list-item interact EventEmitter and updates examples to container-level handling.

Suggested labels

maintenance

Suggested reviewers

jgroth
adrianschmidt
LucyChyzhova

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title accurately describes the primary change of preventing duplicate interact event emissions in the list component after item clicks and uses concise, specific phrasing to highlight the bug fix.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2025-10-15T09:16:02Z

Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3706/

coderabbitai

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/components/list-item/examples/list-item-interactive.tsx (1)

3-35: Update example docs to reflect removal of interact event

The description still says “emits an event with details about the item”. That no longer applies after removing the component’s interact event. Please update the docs accordingly.

src/components/list-item/list-item.tsx (1)

176-183: Remove component-level keyboard handling in limel-list-item

In src/components/list-item/list-item.tsx, drop the import of Element, the @Element host property, the onKeyDown={this.onKeyDown} on <Host>, and the onKeyDown method. Delegate all Enter/Space handling to container components.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d816178 and 9dec4ac.

📒 Files selected for processing (5)

src/components/list-item/examples/list-item-actions.tsx (3 hunks)
src/components/list-item/examples/list-item-checkbox.tsx (2 hunks)
src/components/list-item/examples/list-item-interactive.tsx (3 hunks)
src/components/list-item/examples/list-item-radio.tsx (2 hunks)
src/components/list-item/list-item.tsx (3 hunks)

🧰 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 render method, 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-item/examples/list-item-interactive.tsx
src/components/list-item/examples/list-item-checkbox.tsx
src/components/list-item/examples/list-item-radio.tsx
src/components/list-item/list-item.tsx
src/components/list-item/examples/list-item-actions.tsx