[WEB-2444] improvement: performance improvement for useOutsideClickDetector and usePeekOverviewOutsideClickDetector.#5595
Conversation
…tector` and `usePeekOverviewOutsideClickDetector`. * Move outside click detector to plane helpers package.
prateekshourya29
requested review from
SatishGandham,
rahulramesha and
sriramveeraghanta
WalkthroughThe changes involve restructuring the import statements for the Changes
Possibly related PRs
Suggested labels
Poem
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: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (53)
- admin/core/components/admin-sidebar/root.tsx (1 hunks)
- admin/core/hooks/use-outside-click-detector.tsx (0 hunks)
- admin/next-env.d.ts (1 hunks)
- admin/package.json (1 hunks)
- package.json (1 hunks)
- packages/helpers/hooks/index.ts (1 hunks)
- packages/helpers/hooks/use-outside-click-detector.tsx (1 hunks)
- packages/helpers/index.ts (1 hunks)
- packages/helpers/package.json (1 hunks)
- packages/ui/package.json (1 hunks)
- packages/ui/src/dropdown/multi-select.tsx (1 hunks)
- packages/ui/src/dropdown/single-select.tsx (1 hunks)
- packages/ui/src/dropdowns/context-menu/root.tsx (1 hunks)
- packages/ui/src/dropdowns/custom-menu.tsx (1 hunks)
- packages/ui/src/dropdowns/custom-search-select.tsx (1 hunks)
- packages/ui/src/dropdowns/custom-select.tsx (1 hunks)
- packages/ui/src/emoji/emoji-icon-picker-new.tsx (1 hunks)
- packages/ui/src/emoji/emoji-icon-picker.tsx (1 hunks)
- packages/ui/src/hooks/use-outside-click-detector.tsx (0 hunks)
- space/core/hooks/use-outside-click.tsx (0 hunks)
- web/app/[workspaceSlug]/(projects)/sidebar.tsx (1 hunks)
- web/app/profile/sidebar.tsx (1 hunks)
- web/ce/components/issues/quick-add/root.tsx (1 hunks)
- web/core/components/core/image-picker-popover.tsx (1 hunks)
- web/core/components/cycles/archived-cycles/header.tsx (1 hunks)
- web/core/components/cycles/cycles-view-header.tsx (1 hunks)
- web/core/components/gantt-chart/sidebar/gantt-dnd-HOC.tsx (1 hunks)
- web/core/components/issues/issue-layouts/calendar/issue-block-root.tsx (1 hunks)
- web/core/components/issues/issue-layouts/calendar/issue-block.tsx (1 hunks)
- web/core/components/issues/issue-layouts/gantt/blocks.tsx (1 hunks)
- web/core/components/issues/issue-layouts/kanban/block.tsx (2 hunks)
- web/core/components/issues/issue-layouts/list/block-root.tsx (1 hunks)
- web/core/components/issues/issue-layouts/properties/labels.tsx (1 hunks)
- web/core/components/issues/issue-layouts/spreadsheet/issue-row.tsx (1 hunks)
- web/core/components/issues/select/label.tsx (1 hunks)
- web/core/components/labels/label-block/label-item-block.tsx (1 hunks)
- web/core/components/modules/archived-modules/header.tsx (1 hunks)
- web/core/components/modules/module-view-header.tsx (1 hunks)
- web/core/components/pages/list/search-input.tsx (1 hunks)
- web/core/components/profile/sidebar.tsx (1 hunks)
- web/core/components/project/header.tsx (1 hunks)
- web/core/components/views/view-list-header.tsx (1 hunks)
- web/core/components/workspace/sidebar/favorites/favorite-folder.tsx (1 hunks)
- web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx (1 hunks)
- web/core/components/workspace/sidebar/favorites/new-fav-folder.tsx (1 hunks)
- web/core/components/workspace/sidebar/projects-list-item.tsx (1 hunks)
- web/core/components/workspace/sidebar/workspace-menu.tsx (1 hunks)
- web/core/hooks/use-dropdown.ts (1 hunks)
- web/core/hooks/use-dynamic-dropdown.tsx (1 hunks)
- web/core/hooks/use-outside-click-detector.tsx (0 hunks)
- web/core/hooks/use-peek-overview-outside-click.tsx (2 hunks)
- web/core/store/user/index.ts (1 hunks)
- web/package.json (1 hunks)
Files not reviewed due to no reviewable changes (4)
- admin/core/hooks/use-outside-click-detector.tsx
- packages/ui/src/hooks/use-outside-click-detector.tsx
- space/core/hooks/use-outside-click.tsx
- web/core/hooks/use-outside-click-detector.tsx
Files skipped from review due to trivial changes (23)
- admin/core/components/admin-sidebar/root.tsx
- admin/next-env.d.ts
- packages/helpers/hooks/index.ts
- packages/helpers/index.ts
- packages/ui/src/dropdowns/context-menu/root.tsx
- packages/ui/src/emoji/emoji-icon-picker-new.tsx
- web/app/[workspaceSlug]/(projects)/sidebar.tsx
- web/app/profile/sidebar.tsx
- web/ce/components/issues/quick-add/root.tsx
- web/core/components/core/image-picker-popover.tsx
- web/core/components/cycles/cycles-view-header.tsx
- web/core/components/issues/issue-layouts/calendar/issue-block-root.tsx
- web/core/components/issues/issue-layouts/calendar/issue-block.tsx
- web/core/components/issues/issue-layouts/gantt/blocks.tsx
- web/core/components/issues/issue-layouts/properties/labels.tsx
- web/core/components/issues/select/label.tsx
- web/core/components/profile/sidebar.tsx
- web/core/components/project/header.tsx
- web/core/components/views/view-list-header.tsx
- web/core/components/workspace/sidebar/favorites/favorite-folder.tsx
- web/core/hooks/use-dropdown.ts
- web/core/hooks/use-dynamic-dropdown.tsx
- web/core/store/user/index.ts
Additional comments not posted (29)
packages/helpers/package.json (1)
1-15: LGTM!The
package.jsonfile for the new@plane/helperspackage is properly configured with the necessary metadata and dependencies. The package is marked as private, which aligns with the intention of sharing the helper functions internally across multiple apps.packages/helpers/hooks/use-outside-click-detector.tsx (1)
1-29: LGTM!The
useOutsideClickDetectorhook is correctly implemented and follows the best practices for using theuseEffecthook. The hook uses theclosestmethod to check for the closest element with the attributedata-prevent-outside-click, which is more efficient than usingquerySelectorAll. The hook also uses theuseCaptureparameter to specify whether to use event capturing or event bubbling, which is a nice touch. Finally, the hook uses themousedownevent instead of theclickevent to prevent the callback from being called when the user starts a selection inside the specified element and releases the mouse button outside of it, which is a common gotcha.package.json (1)
17-18: LGTM!The addition of the
"packages/helpers"path to theworkspacesarray is a valid change. It aligns with the PR objective of relocating shared utility functions, such as the outside click detector, to a centralized helpers package. This change will enable better code organization and reuse across the project.admin/package.json (1)
18-18: LGTM!The
@plane/helpersdependency has been added correctly to theadmin/package.jsonfile. This aligns with the PR objective of relocating the outside click detector to the plane helpers package.The dependency addition itself does not introduce any issues. The actual usage of the helpers would be reviewed in the relevant code files.
web/core/hooks/use-peek-overview-outside-click.tsx (3)
10-16: LGTM!The changes optimize the outside click detection by using the
closestmethod to find the nearest ancestor element with thedata-prevent-outside-clickattribute. This approach reduces the number of DOM queries and improves performance compared to the previous implementation. The logic is correct and the code looks good.
27-33: Looks good!The changes streamline the handling of elements with the
data-delay-outside-clickattribute by using theclosestmethod, similar to the optimization made fordata-prevent-outside-click. This improves the efficiency of the code. The timeout for the callback has also been adjusted to 0 milliseconds, which may enhance responsiveness. The implementation is correct and the changes are good to go.
Line range hint
1-9: Rest of the changes look good!The remaining code changes are minor and do not introduce any issues. They are mostly related to formatting and do not affect the functionality of the hook. The code looks clean and well-structured.
Also applies to: 17-26, 34-48
web/package.json (1)
30-30: LGTM!The addition of the
@plane/helpersdependency aligns with the PR objective of relocating the outside click detector to the plane helpers package. The change is straightforward and does not introduce any issues.web/core/components/pages/list/search-input.tsx (1)
3-4: LGTM!The import statement change aligns with the PR objective of relocating the outside click detector to the plane helpers package. Importing from a centralized location can improve code maintainability and reusability. The change is unlikely to impact the component's functionality.
web/core/components/labels/label-block/label-item-block.tsx (1)
5-6: LGTM! Verify the new import works as expected.Moving the
useOutsideClickDetectorhook to a shared library (@plane/helpers) is a good refactoring decision that can improve maintainability and consistency across the codebase. The change itself looks straightforward.However, please ensure that the new import works as expected and doesn't introduce any unintended side effects. Verify that the behavior of the
LabelItemBlockcomponent remains unchanged after this modification.Run the following script to verify the hook usage:
Verification successful
Refactoring Verified: Import Path Updated Successfully
The refactoring to import
useOutsideClickDetectorfrom@plane/helpershas been applied consistently across the codebase. The old import path is no longer present, and the usage of the hook remains unchanged, indicating that the refactoring did not introduce any issues.
- The old import path
@/hooks/use-outside-click-detectoris not found.- The new import path
@plane/helpersis used extensively and correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `useOutsideClickDetector` have been updated to the new import. # Test 1: Search for the old import. Expect: No results. rg --type typescript $'@/hooks/use-outside-click-detector' # Test 2: Search for the new import. Expect: Only the new import statement. rg --type typescript $'@plane/helpers' # Test 3: Search for the hook usage. Expect: No changes in the usage. rg --type typescript -A 5 $'useOutsideClickDetector'Length of output: 252
Script:
#!/bin/bash # Description: Verify all usages of `useOutsideClickDetector` have been updated to the new import. # Test 1: Search for the old import. Expect: No results. rg --type ts $'@/hooks/use-outside-click-detector' # Test 2: Search for the new import. Expect: Only the new import statement. rg --type ts $'@plane/helpers' # Test 3: Search for the hook usage. Expect: No changes in the usage. rg --type ts -A 5 $'useOutsideClickDetector'Length of output: 42947
web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx (1)
7-8: Verify the hook implementation in the new location.The import statement change looks good. However, please ensure that the implementation of
useOutsideClickDetectorin@plane/helpersis consistent with its previous behavior to avoid any regressions.Run the following script to compare the hook implementations:
web/core/components/workspace/sidebar/favorites/new-fav-folder.tsx (1)
5-6: LGTM!The import statement update for
useOutsideClickDetectoris a good refactor that centralizes the hook in the@plane/helperspackage. This change improves code organization and reusability of the hook across different components, which aligns with the PR objectives.web/core/components/gantt-chart/sidebar/gantt-dnd-HOC.tsx (1)
8-8: Verify the behavior of theuseOutsideClickDetectorhook.The change in the import statement for
useOutsideClickDetectorsuggests a refactor or consolidation of utility hooks within the project. While the overall structure and functionality of the component remain intact, it's important to verify that the behavior of theuseOutsideClickDetectorhook remains consistent with its previous implementation.Run the following script to verify the behavior of the
useOutsideClickDetectorhook:Verification successful
The behavior of the
useOutsideClickDetectorhook is consistent and correct.The
useOutsideClickDetectorhook is implemented as an exported constant inpackages/helpers/hooks/use-outside-click-detector.tsxand is used extensively across the codebase. The centralized import from@plane/helperssuggests a refactor for better organization. The behavior of the hook remains consistent with its previous implementation.
- Implementation found in:
packages/helpers/hooks/use-outside-click-detector.tsx- Extensive usage across multiple files confirms its consistent behavior.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the behavior of the `useOutsideClickDetector` hook. # Test 1: Search for the hook implementation. Expect: The hook implementation should exist in the `@plane/helpers` module. ast-grep --lang typescript --pattern $'function useOutsideClickDetector($_) { $$$ }' # Test 2: Search for the hook usage. Expect: The hook should be used consistently across the codebase. rg --type typescript $'useOutsideClickDetector'Length of output: 169
Script:
#!/bin/bash # Description: Verify the behavior of the `useOutsideClickDetector` hook. # Test 1: Search for the hook implementation. Expect: The hook implementation should exist in the `@plane/helpers` module. # Check for different possible implementations, including arrow functions. ast-grep --lang typescript --pattern $'const useOutsideClickDetector = ($_) => { $$$ }' # Test 2: Search for the hook usage. Expect: The hook should be used consistently across the codebase. rg 'useOutsideClickDetector' --type-add 'ts:*.{ts,tsx}' --type tsLength of output: 8414
packages/ui/src/emoji/emoji-icon-picker.tsx (1)
5-6: LGTM!The import statement update aligns with the PR objectives and the refactoring effort to consolidate utility functions. Importing the
useOutsideClickDetectorhook from the@plane/helperspackage can improve code maintainability and reusability.packages/ui/src/dropdown/multi-select.tsx (1)
7-8: Verify the behavior of the migrateduseOutsideClickDetectorhook.The change in the import statement for
useOutsideClickDetectoris good as it consolidates the utility function in a centralized location. However, please ensure that the migrated hook from@plane/helpershas the same behavior as the previous local implementation to avoid any regressions.Run the following script to verify the behavior of the migrated hook:
packages/ui/src/dropdown/single-select.tsx (1)
7-8: LGTM!The change aligns with the PR objective of migrating the
useOutsideClickDetectorhook to a centralized location in the@plane/helpersmodule. This refactor improves the maintainability and reusability of the hook by moving it to a shared location.web/core/components/cycles/archived-cycles/header.tsx (1)
6-7: LGTM!The change consolidates the import path for
useOutsideClickDetectorto a centralized location in@plane/helpers. This is a good refactor that improves code organization and maintainability without altering the existing behavior of the component.packages/ui/src/dropdowns/custom-select.tsx (1)
5-6: Verify the impact of the import change on the dropdown functionality.The change in the import statement for the
useOutsideClickDetectorhook is consistent with the PR objective of relocating the outside click detector to the plane helpers package. However, please ensure that this change does not introduce any unintended side effects or impact the functionality of the dropdown component.Run the following script to verify the functionality of the dropdown component:
Verification successful
Import change verified successfully.
The import of
useOutsideClickDetectorfrom@plane/helpersis consistent across the codebase, including in thepackages/ui/src/dropdowns/custom-select.tsxfile. This confirms that the refactor to centralize this utility function is correctly implemented without any negative impact on the dropdown component's functionality.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the functionality of the dropdown component. # Test 1: Search for the usage of the `useOutsideClickDetector` hook in the dropdown component. # Expect: The hook is being used correctly. ast-grep --lang typescript --pattern $'useOutsideClickDetector($_)' # Test 2: Search for any errors or warnings related to the `useOutsideClickDetector` hook. # Expect: No errors or warnings. rg --type typescript $'useOutsideClickDetector' -A 5 | rg --color never $'error|warning'Length of output: 190
Script:
#!/bin/bash # Description: Verify the functionality of the dropdown component. # Test 1: Search for the usage of the `useOutsideClickDetector` hook in the dropdown component. # Expect: The hook is being used correctly. ast-grep --lang typescript --pattern $'useOutsideClickDetector($_)' # Test 2: Search for any errors or warnings related to the `useOutsideClickDetector` hook. # Expect: No errors or warnings. rg --type ts $'useOutsideClickDetector' -A 5 | rg --color never $'error|warning'Length of output: 147
Script:
#!/bin/bash # Description: Verify the presence and definition of the `useOutsideClickDetector` hook. # Search for the definition of the `useOutsideClickDetector` hook in the codebase. ast-grep --lang typescript --pattern $'function useOutsideClickDetector($_) { $$$ }'Length of output: 85
Script:
#!/bin/bash # Description: Verify the import of the `useOutsideClickDetector` hook. # Search for import statements of the `useOutsideClickDetector` hook in the codebase. rg 'import { useOutsideClickDetector' --type tsLength of output: 173
Script:
#!/bin/bash # Description: Verify the import of the `useOutsideClickDetector` hook. # Search for import statements of the `useOutsideClickDetector` hook in the codebase. rg 'import \{ useOutsideClickDetector' --type tsLength of output: 4101
web/core/components/modules/archived-modules/header.tsx (1)
6-7: LGTM!The import path change for
useOutsideClickDetectoris part of a larger refactor to consolidate imports and does not introduce any functional changes to the component. This is a good practice for better code organization and maintainability.web/core/components/issues/issue-layouts/list/block-root.tsx (1)
8-9: LGTM!The import statement change for
useOutsideClickDetectorlooks good. Moving the hook to the@plane/helpersmodule is a sensible refactoring to improve code organization by centralizing reusable utilities.To ensure the component behavior remains unaffected, please verify that the
useOutsideClickDetectorhook from@plane/helpersfunctions equivalently to the previous implementation. You can run the following script to search for the hook usage and compare the implementations:Verification successful
Refactoring Verified:
useOutsideClickDetectorHook Implementation is Consistent and CentralizedThe
useOutsideClickDetectorhook has been successfully refactored to a centralized location in@plane/helpers, maintaining its functionality across the codebase. The implementation aligns with the expected behavior of detecting outside clicks and executing a callback, ensuring that component behavior remains unaffected by this change.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify `useOutsideClickDetector` hook usage and implementation. # Test 1: Search for the hook usage in the codebase. # Expect: The hook should be used consistently across components. rg --type typescript -A 5 $'useOutsideClickDetector' # Test 2: Compare the new and old hook implementations. # Expect: The implementations should be functionally equivalent. # Search for the new hook implementation ast-grep --lang typescript --pattern $'function useOutsideClickDetector($_) { $$$ }' # Search for the old hook implementation ast-grep --lang typescript --pattern $'function useOutsideClickDetector($_) { $$$ }'Length of output: 262
Script:
#!/bin/bash # Description: Verify `useOutsideClickDetector` hook usage and implementation. # Test 1: Search for the hook usage in the codebase. # Correct the file type to `ts` for TypeScript files. rg --type ts -A 5 'useOutsideClickDetector' # Test 2: Compare the new and old hook implementations. # Adjust the pattern to match potential variations in function definition. ast-grep --lang typescript --pattern $'function useOutsideClickDetector($$$) { $$$ }'Length of output: 38905
Script:
#!/bin/bash # Description: Extract the implementation of `useOutsideClickDetector` from the helpers package. # Extract the implementation of the hook from the specified file. rg --type ts -A 10 'export const useOutsideClickDetector' packages/helpers/hooks/use-outside-click-detector.tsxLength of output: 587
web/core/components/modules/module-view-header.tsx (2)
11-11: LGTM!Moving the
useOutsideClickDetectorhook to the shared@plane/helperspackage is a good refactor that promotes code reuse and maintainability. This change does not introduce any new functionality or alter the existing behavior of the component.
8-8: Also applies to: 10-10packages/ui/src/dropdowns/custom-menu.tsx (1)
6-7: Verify the behavior of the imported hook.Moving
useOutsideClickDetectorto a shared helpers package is a good refactor that promotes code reuse and maintainability. Consolidating common utilities in a central location reduces duplication and makes it easier to update the hook across the codebase.Ensure that the imported hook from
@plane/helpershas equivalent behavior to the previous local implementation to avoid introducing any regressions. You can run the following script to compare the implementations:Verification successful
Behavior of the imported hook is equivalent to the previous local implementation.
The implementation of
useOutsideClickDetectorin the shared helpers package is identical to the previous local implementation. This confirms that the refactor does not introduce any regressions or changes in functionality. The move to a shared package successfully promotes code reuse and maintainability without affecting the component's behavior.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the imported hook has equivalent behavior to the previous local implementation. # Test: Search for the previous and new hook implementations. Expect: Matching logic between the two. echo "Previous local implementation:" rg --type typescript -A 10 $'export (const|function) useOutsideClickDetector' echo "New implementation in @plane/helpers:" rg --type typescript -A 10 $'export (const|function) useOutsideClickDetector' $(fd --type directory --hidden --exclude '{node_modules,build,dist}' helpers)Length of output: 996
Script:
#!/bin/bash # Description: Verify the imported hook has equivalent behavior to the previous local implementation. # Test: Search for the previous and new hook implementations. Expect: Matching logic between the two. echo "Previous local implementation:" rg --type ts -A 10 $'export (const|function) useOutsideClickDetector' echo "New implementation in @plane/helpers:" rg --type ts -A 10 $'export (const|function) useOutsideClickDetector' $(fd --type directory --hidden --exclude '{node_modules,build,dist}' helpers)Length of output: 3046
packages/ui/src/dropdowns/custom-search-select.tsx (1)
6-7: Verify the implementation of theuseOutsideClickDetectorhook.The refactor to import the
useOutsideClickDetectorhook from the centralized@plane/helpersmodule looks good. However, to ensure that the component continues to function as expected, please verify that the implementation of the hook in the@plane/helpersmodule matches the previous local implementation.Run the following script to compare the implementations:
web/core/components/workspace/sidebar/workspace-menu.tsx (1)
9-10: Verify the hook relocation.The refactor looks good! Moving the
useOutsideClickDetectorhook to the shared@plane/helperspackage aligns with the PR objective and can help streamline its usage across the application.Please confirm that the hook implementation has been correctly moved to the new
@plane/helperspackage. You can use the following script to verify:Verification successful
Hook relocation verified.
The
useOutsideClickDetectorhook has been successfully moved to the@plane/helperspackage, as confirmed by its definition in./packages/helpers/hooks/use-outside-click-detector.tsx. This aligns with the refactor objective and ensures consistent usage across the application.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify `useOutsideClickDetector` hook has been moved to `@plane/helpers`. # Test: Search for the hook declaration. # Expect: Hook declaration found only in `@plane/helpers`. rg --type typescript -g '!node_modules' -g '!.next' $'useOutsideClickDetector\s*\('Length of output: 119
Script:
#!/bin/bash # Description: Verify `useOutsideClickDetector` hook has been moved to `@plane/helpers`. # Find all TypeScript files and search for the hook declaration. fd --extension ts --extension tsx --exclude node_modules --exclude .next --exec rg 'useOutsideClickDetector\s*\('Length of output: 2261
Script:
#!/bin/bash # Description: Verify the definition of `useOutsideClickDetector` is in `@plane/helpers`. # Search for the definition of the hook in the `@plane/helpers` package. fd --extension ts --extension tsx --exclude node_modules --exclude .next --exec rg 'function useOutsideClickDetector|const useOutsideClickDetector'Length of output: 190
Script:
#!/bin/bash # Description: Find the file path of `useOutsideClickDetector` definition. # Search for the file containing the definition of the hook. fd --extension ts --extension tsx --exclude node_modules --exclude .next --exec rg --files-with-matches 'export const useOutsideClickDetector'Length of output: 200
web/core/components/issues/issue-layouts/kanban/block.tsx (2)
8-9: LGTM!The change to import the
useOutsideClickDetectorhook from@plane/helpersis a good refactor to centralize helper functions. It does not affect the functionality of the component.
191-191: Verify the usage of the ID attribute.Adding an ID attribute to the issue block div element is a good change that may facilitate better DOM manipulation or event handling. Please ensure that the added ID attribute is being used appropriately in the codebase, such as in event handlers or selectors.
Run the following script to verify the usage of the ID attribute:
web/core/components/issues/issue-layouts/spreadsheet/issue-row.tsx (1)
7-8: LGTM!The refactoring of the
useOutsideClickDetectorimport to use the centralized@plane/helperspackage is a good change. It improves code organization and reusability without altering the core functionality of the component.web/core/components/workspace/sidebar/projects-list-item.tsx (1)
26-27: Refactor looks good!The import statement has been updated to use the
useOutsideClickDetectorhook from the@plane/helperspackage. This is a good refactor to centralize the hook in a common helpers package.
| { | ||
| "name": "@plane/helpers", | ||
| "version": "0.22.0", | ||
| "description": "Helper functions shared across multiple apps internally", | ||
| "main": "index.ts", | ||
| "private": true, | ||
| "devDependencies": { | ||
| "@types/node": "^22.5.4", | ||
| "@types/react": "^18.3.5", | ||
| "typescript": "^5.6.2" | ||
| }, | ||
| "dependencies": { | ||
| "react": "^18.3.1" | ||
| } | ||
| } |
There was a problem hiding this comment.
Tip
Codebase Verification
Old implementation of useOutsideClickDetector still exists.
The useOutsideClickDetector hook is being correctly imported from the @plane/helpers package across the codebase. However, the old implementation of the hook still exists in the following file and should be removed:
packages/helpers/hooks/use-outside-click-detector.tsx
Analysis chain
Verify the usage of the @plane/helpers package.
To ensure that the new @plane/helpers package is being utilized correctly, please verify that:
- The package is imported and used in the intended locations across the codebase.
- The imports for the
useOutsideClickDetectorhook have been updated to use the new package. - The old implementations of the hook have been removed.
Run the following script to verify the package usage:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of the `@plane/helpers` package.
# Test 1: Search for imports of the package. Expect: Imports in the intended locations.
rg --type typescript $'@plane/helpers'
# Test 2: Search for imports of the `useOutsideClickDetector` hook. Expect: Imports from the new package.
rg --type typescript $'useOutsideClickDetector'
# Test 3: Search for old implementations of the hook. Expect: No occurrences.
rg --type typescript $'const useOutsideClickDetector'
Length of output: 244
Script:
#!/bin/bash
# Description: Verify the usage of the `@plane/helpers` package.
# Test 1: Search for imports of the package. Expect: Imports in the intended locations.
rg '@plane/helpers'
# Test 2: Search for imports of the `useOutsideClickDetector` hook. Expect: Imports from the new package.
rg 'useOutsideClickDetector'
# Test 3: Search for old implementations of the hook. Expect: No occurrences.
rg 'const useOutsideClickDetector'
Length of output: 12689
| "@blueprintjs/popover2": "^1.13.3", | ||
| "@headlessui/react": "^1.7.3", | ||
| "@popperjs/core": "^2.11.8", | ||
| "@plane/helpers": "*", |
There was a problem hiding this comment.
Tip
Codebase Verification
Remove unused dependency @plane/helpers.
The @plane/helpers dependency added to the packages/ui/package.json file does not appear to be used in the codebase, as no import statements were found. It is advisable to remove this dependency to prevent unnecessary increases in package size and complexity.
- File:
packages/ui/package.json - Line: 29
Analysis chain
Verify the usage and version specification of the new dependency.
I noticed that a new dependency @plane/helpers has been added to the package with a version specification of "*". While this addition could potentially enhance the functionality of the package, it's important to consider the following:
-
Ensure that the new dependency is actually being used within the package and that it aligns with the package's functionality and requirements. Unused dependencies can unnecessarily increase the package size and complexity.
-
Consider specifying a more precise version range for the dependency to avoid potential inconsistencies or breaking changes that may occur with future updates. Using a wildcard version specification ("*") can introduce instability and make it harder to reproduce builds.
To verify the usage of the new dependency, you can run the following script:
Please let me know if you have any questions or if you would like me to assist with updating the dependency specification.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description:
# - Verify if the new dependency `@plane/helpers` is being used within the package.
# - Check if the version specification can be updated to a more precise range.
# Test 1: Search for imports from `@plane/helpers` in the package source files.
# Expect: At least one import statement from `@plane/helpers`.
echo "Searching for imports from @plane/helpers..."
rg --type typescript --type javascript --glob '!dist' --glob '!node_modules' -e 'from\s+["'\'']@plane/helpers'
# Test 2: Check the latest available version of `@plane/helpers`.
# Expect: A specific version number.
echo "Checking the latest available version of @plane/helpers..."
npm view @plane/helpers version
# Test 3: Check if the package has any dependencies that also depend on `@plane/helpers`.
# Expect: No conflicting dependency versions.
echo "Checking for potential version conflicts in the dependency tree..."
npm ls @plane/helpers
Length of output: 718
Script:
#!/bin/bash
# Description:
# - Verify if the new dependency `@plane/helpers` is being used within the package.
# Test: Search for imports from `@plane/helpers` in the package source files.
# Expect: At least one import statement from `@plane/helpers`.
echo "Searching for imports from @plane/helpers using ast-grep..."
ast-grep --lang javascript --pattern 'import $_ from "@plane/helpers"' || ast-grep --lang typescript --pattern 'import $_ from "@plane/helpers"'
Length of output: 201
3 participants
Improvements
This PR optimizes
useOutsideClickDetectorandusePeekOverviewOutsideClickDetectorby replacingquerySelectorAllwith a more efficient check for the closest element with the required attribute on the event target. This enhances the performance of the outside click detection.Media
Before
After
Other changes
issue_[issue-id]tokanbanandganttlayout issue blocks to prevent PeekOverview from closing when the same issue is clicked.Summary by CodeRabbit
New Features
@plane/helpers, enhancing modularity and functionality.useOutsideClickDetector, for improved outside click detection in various components.Bug Fixes
useOutsideClickDetector, streamlining the import structure.Documentation
useOutsideClickDetectorto reflect the new centralized helper module.Chores