feat(menu): enable keeping the menu open after selection#4004
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 22 minutes and 36 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a new boolean prop Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-4004/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/menu/examples/menu-keep-open.tsx`:
- Around line 161-164: The code pushes selectedItem.value.propertyName (typed
string | undefined) into appliedFilters (string[]), causing a type-safety
mismatch; update the logic around appliedFilters (where you modify
this.appliedFilters) to ensure propertyName is narrowed or guarded before
adding: check selectedItem.value.propertyName is a string (e.g., if
(selectedItem.value.propertyName) ...) or assert its presence when applyAll is
false (use a non-null assertion only if certain from handleSearch), so that
appliedFilters only ever receives a string; reference the variables
appliedFilters, selectedItem.value.propertyName, applyAll, and handleSearch when
making the change.
In `@src/components/menu/menu.tsx`:
- Around line 192-196: Update the JSDoc for the public property keepOpenOnSelect
to state that the menu only remains open after selection when a searcher is
configured and the current searchValue is non-empty; reference the component's
searcher and searchValue behavior so readers know that without a searcher or
when searchValue is empty the menu will close normally.
- Around line 781-793: The refreshSearch method can leave loadingSubItems true
if this.searcher(query) throws; wrap the await this.searcher(query) call in a
try/catch/finally so errors are caught, loadingSubItems is set back to false in
the finally block, and any error is logged or handled in the catch; ensure you
still check this.searchValue !== query before assigning this.searchResults or
calling this.setFocus so cancelled searches don't overwrite state. Use the
existing symbols refreshSearch, this.searcher, loadingSubItems, searchValue,
searchResults, and setFocus to implement the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cb89ff82-9aaa-4b6e-b079-c53b3a16711c
⛔ Files ignored due to path filters (1)
etc/lime-elements.api.mdis excluded by!etc/lime-elements.api.md
📒 Files selected for processing (2)
src/components/menu/examples/menu-keep-open.tsxsrc/components/menu/menu.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/menu/examples/menu-keep-open.tsx`:
- Around line 8-10: The current isApple detection relies on
navigator.userAgentData?.platform with a fallback to deprecated
navigator.platform; update the detection to be more robust by preferring
navigator.userAgentData?.platform, then falling back to navigator.userAgent
(inspect the userAgent string for Mac/iPhone/iPad/iPod), and only use
navigator.platform as the final fallback; update the const isApple declaration
so it checks these sources in that order (refer to the existing isApple constant
and its userAgent/userAgentData/platform references).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f44ae042-9362-4181-bb5d-56cb1250dea9
⛔ Files ignored due to path filters (1)
etc/lime-elements.api.mdis excluded by!etc/lime-elements.api.md
📒 Files selected for processing (2)
src/components/menu/examples/menu-keep-open.tsxsrc/components/menu/menu.tsx
adrianschmidt
left a comment
adrianschmidt
left a comment
There was a problem hiding this comment.
External API approved.
adrianschmidt
force-pushed
the
menu-keep-open
branch
from
83d7b7e to
7ac69e4
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/components/menu/menu.tsx (2)
781-793: 🧹 Nitpick | 🔵 TrivialConsider adding error handling for resilience.
If
this.searcher(query)throws,loadingSubItemsremainstrue, leaving the menu in a permanent loading state. While this mirrors the existinghandleTextInputbehavior (lines 626-630), adding try/finally would improve robustness.🛡️ Optional: Add error handling
private readonly refreshSearch = async () => { const query = this.searchValue; this.loadingSubItems = true; + try { const result = await this.searcher(query); if (this.searchValue !== query) { return; } this.searchResults = result; + } finally { this.loadingSubItems = false; + } this.setFocus(); };,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/menu/menu.tsx` around lines 781 - 793, The refreshSearch method can leave loadingSubItems true if this.searcher(query) throws; wrap the await this.searcher(query) call in a try/finally so loadingSubItems is always set to false, and keep the existing stale-query check (this.searchValue !== query) and the setFocus() call only after the finally block (or inside it after confirming the query is still current). Also optionally catch/log the error from this.searcher to avoid unhandled rejections. Ensure you modify the refreshSearch function (referenced by name) and mirror the defensive pattern used in handleTextInput for consistency.
192-196: 🧹 Nitpick | 🔵 TrivialDocumentation should clarify the searcher requirement.
The JSDoc says the menu stays open after selection, but reviewing lines 938-944 reveals this only applies when a
searcheris provided ANDsearchValueis non-empty. Without these conditions, the menu closes normally.📝 Suggested documentation improvement
/** - * When `true`, the menu stays open after an item is selected. + * When `true`, the menu stays open after an item is selected, + * provided that a `searcher` is configured and the search field + * contains a query. In this case, the search is re-executed so + * the result list can be updated (e.g. to exclude the selected item). */ `@Prop`({ reflect: true }) public keepOpenOnSelect = false;,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/menu/menu.tsx` around lines 192 - 196, Update the JSDoc for the keepOpenOnSelect Prop to reflect the actual behavior: state that it only keeps the menu open after selection when a searcher is configured and the component's searchValue is non-empty (otherwise the menu closes normally); reference the keepOpenOnSelect property and the searcher and searchValue conditions to make the requirement explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/menu/menu.tsx`:
- Around line 781-793: The refreshSearch method can leave loadingSubItems true
if this.searcher(query) throws; wrap the await this.searcher(query) call in a
try/finally so loadingSubItems is always set to false, and keep the existing
stale-query check (this.searchValue !== query) and the setFocus() call only
after the finally block (or inside it after confirming the query is still
current). Also optionally catch/log the error from this.searcher to avoid
unhandled rejections. Ensure you modify the refreshSearch function (referenced
by name) and mirror the defensive pattern used in handleTextInput for
consistency.
- Around line 192-196: Update the JSDoc for the keepOpenOnSelect Prop to reflect
the actual behavior: state that it only keeps the menu open after selection when
a searcher is configured and the component's searchValue is non-empty (otherwise
the menu closes normally); reference the keepOpenOnSelect property and the
searcher and searchValue conditions to make the requirement explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 48c4857b-b9cb-415e-a925-9e817d51f1fe
⛔ Files ignored due to path filters (1)
etc/lime-elements.api.mdis excluded by!etc/lime-elements.api.md
📒 Files selected for processing (2)
src/components/menu/examples/menu-keep-open.tsxsrc/components/menu/menu.tsx
adrianschmidt
force-pushed
the
menu-keep-open
branch
from
7ac69e4 to
3632d84
Compare
|
🎉 This PR is included in version 39.13.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
fix https://github.com/Lundalogik/crm-client/issues/864
Summary by CodeRabbit
New Features
Documentation
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: