feat: Add useActiveDoubleClick hook for improved double-click handling #39474
Conversation
Introduces a new useActiveDoubleClick hook to manage double-click interactions for editable components. The hook provides a more controlled double-click experience by: - Enabling double-click only when an item is active - Preventing rapid or unintended double-clicks - Simplifying double-click logic across different components
WalkthroughThis pull request introduces a new custom hook, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Component
participant H as useActiveDoubleClick Hook
participant CB as Callback
U->>C: Double-click event
C->>H: Call hook with active state & callback
H-->>H: Evaluate canDoubleClick (via state/effect)
alt Double-click enabled
H->>CB: Execute callback (e.g., enter edit mode)
else
H->>C: Return noop function
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (9)
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
Documentation and Community
|
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13563563851. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/client/packages/design-system/ads/src/__hooks__/useActiveDoubleClick.ts (1)
18-20: Consider making the timeout delay configurable or extracting it to a constant.The hardcoded 200ms value would be more maintainable as a named constant or configurable parameter.
+ const DOUBLE_CLICK_DELAY = 200; // ms + export function useActiveDoubleClick( isActive: boolean, onDoubleClick?: () => void, + delayMs: number = DOUBLE_CLICK_DELAY, ) { // ... useEffect( function handleDoubleClickEnableBasedOnSelection() { if (isActive) { setTimeout(() => { setCanDoubleClick(); - }, 200); + }, delayMs); } else { setCannotDoubleClick(); } }, - [isActive, setCanDoubleClick, setCannotDoubleClick], + [isActive, setCanDoubleClick, setCannotDoubleClick, delayMs], ); // ... }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/client/packages/design-system/ads/src/Templates/EditableDismissibleTab/EditableDismissibleTab.tsx(2 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityItem/EntityItem.tsx(2 hunks)app/client/packages/design-system/ads/src/__hooks__/index.ts(1 hunks)app/client/packages/design-system/ads/src/__hooks__/useActiveDoubleClick.ts(1 hunks)app/client/packages/design-system/ads/src/__hooks__/useEditableText/useEditableText.ts(0 hunks)
💤 Files with no reviewable changes (1)
- app/client/packages/design-system/ads/src/hooks/useEditableText/useEditableText.ts
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
- GitHub Check: chromatic-deployment
- GitHub Check: storybook-tests
- GitHub Check: chromatic-deployment
🔇 Additional comments (8)
app/client/packages/design-system/ads/src/__hooks__/index.ts (1)
3-3: Added export for the new hook looks good.This export makes the
useActiveDoubleClickhook available for import across the application.app/client/packages/design-system/ads/src/Templates/EditableDismissibleTab/EditableDismissibleTab.tsx (3)
9-9: Import of the new hook looks good.Correctly imports the
useActiveDoubleClickhook from the hooks directory.
38-43: Good implementation of the useActiveDoubleClick hook.The hook is correctly configured with the component's active state and edit mode handler.
43-43: Clean refactoring of the double-click handler.This simplifies the conditional logic by leveraging the hook's functionality.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityItem/EntityItem.tsx (3)
6-6: Import of the new hook looks good.Correctly imports the
useActiveDoubleClickhook from the hooks directory.
9-14: Good implementation of the useActiveDoubleClick hook.The hook is correctly configured with the component's selected state and double-click handler. The null check with
|| falseis a good safety measure.
70-70: Correctly applies the double-click override.The ListItem now uses the enhanced double-click handler from the hook.
app/client/packages/design-system/ads/src/__hooks__/useActiveDoubleClick.ts (1)
1-14: Well-structured hook with appropriate dependencies.The hook correctly manages state using
useBooleanand has a clear function signature.
app/client/packages/design-system/ads/src/__hooks__/useActiveDoubleClick.ts
Outdated
Show resolved
Hide resolved
app/client/packages/design-system/ads/src/__hooks__/useActiveDoubleClick.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/packages/design-system/ads/src/__hooks__/useActiveDoubleClick.ts (1)
1-39: Simplify the handleDoubleClick function logic.The hook implementation is generally good, but the conditional logic in the handleDoubleClick function can be simplified.
const handleDoubleClick = useMemo(() => { if (!canDoubleClick || !onDoubleClick) { return noop; } - if (canDoubleClick) { - return onDoubleClick; - } + return onDoubleClick; }, [canDoubleClick, onDoubleClick]);The second check for
canDoubleClickis redundant since we already confirmed it's true in the first condition.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/client/packages/design-system/ads/src/Templates/EditableDismissibleTab/EditableDismissibleTab.tsx(2 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityItem/EntityItem.tsx(2 hunks)app/client/packages/design-system/ads/src/__hooks__/index.ts(1 hunks)app/client/packages/design-system/ads/src/__hooks__/useActiveDoubleClick.ts(1 hunks)app/client/packages/design-system/ads/src/__hooks__/useEditableText/useEditableText.ts(0 hunks)
💤 Files with no reviewable changes (1)
- app/client/packages/design-system/ads/src/hooks/useEditableText/useEditableText.ts
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
- GitHub Check: chromatic-deployment
- GitHub Check: storybook-tests
- GitHub Check: chromatic-deployment
🔇 Additional comments (7)
app/client/packages/design-system/ads/src/__hooks__/index.ts (1)
3-3: Export addition looks good.Clean addition of the new hook export that follows the established pattern.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityItem/EntityItem.tsx (3)
6-6: Appropriate import added.The hook import is correctly added from the hooks directory.
9-14: Good implementation of destructuring and hook usage.Clean implementation that destructures the necessary props early and correctly uses the hook.
70-70: Correct usage of the doubleClickOverride function.The hook-generated function is properly assigned to the onDoubleClick prop.
app/client/packages/design-system/ads/src/Templates/EditableDismissibleTab/EditableDismissibleTab.tsx (2)
9-9: Appropriate import added.The hook import is correctly added from the hooks directory.
38-43: Good implementation of the hook.The hook is correctly used with the active state and enter edit mode function.
app/client/packages/design-system/ads/src/__hooks__/useActiveDoubleClick.ts (1)
15-26: Consider the timing implications of the timeout.The 200ms delay before enabling double-clicks after an item becomes active creates a small window where double-clicks won't work immediately after selection. This is likely intentional to prevent accidental double-clicks, but worth verifying this behavior is expected.
|
Deploy-Preview-URL: https://ce-39474.dp.appsmith.com |
github-actions
bot
added
ADS Components
| [isActive, setCanDoubleClick, setCannotDoubleClick], | ||
| ); | ||
|
|
||
| const handleDoubleClick = useMemo(() => { |
There was a problem hiding this comment.
It doesn't look like you need useMemo here. Callback onDoubleClick and noop are already stable.
There was a problem hiding this comment.
Merging in for now. It does not harm to keep this in.
Description
Introduces a new useActiveDoubleClick hook to manage double-click interactions for editable components. The hook provides a more controlled double-click experience by:
Fixes #39477
Automation
/ok-to-test tags="@tag.IDE"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13565291252
Commit: 2b591c2
Cypress dashboard.
Tags:
@tag.IDESpec:
Thu, 27 Feb 2025 11:57:50 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit