fix: fixed the issues of repeated action triggering #6597
Conversation
imyuanx
requested review from
ezailWang,
loatheb,
sidmorizon,
originalix and
huhuanming
as code owners
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
WalkthroughThe changes modify the Changes
Sequence DiagramsequenceDiagram
participant User
participant ActionListFrame
participant ActionList
User->>ActionListFrame: Trigger open
alt Not Processing
ActionListFrame->>ActionListFrame: Set isProcessing = true
ActionListFrame->>ActionList: Open action list
ActionListFrame->>ActionListFrame: Reset isProcessing after 350ms
else Processing
Note over ActionListFrame: Ignore additional open requests
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/components/src/actions/ActionList/index.tsx (1)
Line range hint
356-364: Add error handling for async operationThe async call to
estimatedContentHeight()lacks error handling. Add a catch block to handle failures and reset the processing state.if (estimatedContentHeight) { - void estimatedContentHeight().then((height) => { - showActionList({ - ...popoverProps, - estimatedContentHeight: height, - }); - }); + void estimatedContentHeight() + .then((height) => { + showActionList({ + ...popoverProps, + estimatedContentHeight: height, + }); + }) + .catch((error) => { + console.error('Failed to estimate content height:', error); + isProcessing.current = false; + }); } else { showActionList(popoverProps); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/components/src/actions/ActionList/index.tsx(3 hunks)
🔇 Additional comments (1)
packages/components/src/actions/ActionList/index.tsx (1)
347-347: Good choice using useRef!Using
useRefinstead ofuseStateprevents unnecessary re-renders while tracking the processing state.
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/components/src/actions/ActionList/index.tsx (1)
Line range hint
359-377: Consider clearing the timer.
If users close the list earlier or navigate away, you could cancel the reset timer to avoid lingering side effects.const handleActionListOpen = () => { if (isProcessing.current) return; isProcessing.current = true; + const timerId = setTimeout(() => { isProcessing.current = false; - }, PROCESSING_RESET_DELAY); + }, PROCESSING_RESET_DELAY); // Example cancel approach if you have a close event // handleActionListClose = () => { // clearTimeout(timerId); // isProcessing.current = false; // }; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/components/src/actions/ActionList/index.tsx(5 hunks)
🔇 Additional comments (5)
packages/components/src/actions/ActionList/index.tsx (5)
2-2: Nice inclusion ofuseRef.
It'll help prevent extra re-renders while tracking the processing state.
4-4: Good call on importingdebounce.
This helps control repetitive triggers and keeps the code simple.
45-47: Thanks for naming this constant.
This makes the timeout more explicit and easier to maintain.
344-347: Confirm the debounce behavior.
By default,debouncetriggers on the trailing edge. Check if that meets your needs or if you want a leading edge call.
355-355: Smart move using a ref.
This avoids unwanted re-renders while tracking state across renders.
fixed:
https://github.com/user-attachments/assets/8d5cf227-af66-4216-8aaf-3b3fad840096
Summary by CodeRabbit
The changes ensure smoother interaction with the action list, reducing potential performance issues and unexpected behaviors during quick interactions.