-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[WEB-2444] improvement: performance improvement for useOutsideClickDetector and usePeekOverviewOutsideClickDetector.
#5595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from "./use-outside-click-detector"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import React, { useEffect } from "react"; | ||
|
|
||
| export const useOutsideClickDetector = ( | ||
| ref: React.RefObject<HTMLElement>, | ||
| callback: () => void, | ||
| useCapture = false | ||
| ) => { | ||
| const handleClick = (event: MouseEvent) => { | ||
| if (ref.current && !ref.current.contains(event.target as Node)) { | ||
| // check for the closest element with attribute name data-prevent-outside-click | ||
| const preventOutsideClickElement = ( | ||
| event.target as HTMLElement | undefined | ||
| )?.closest("[data-prevent-outside-click]"); | ||
| // if the closest element with attribute name data-prevent-outside-click is found, return | ||
| if (preventOutsideClickElement) { | ||
| return; | ||
| } | ||
| // else call the callback | ||
| callback(); | ||
| } | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| document.addEventListener("mousedown", handleClick, useCapture); | ||
| return () => { | ||
| document.removeEventListener("mousedown", handleClick, useCapture); | ||
| }; | ||
| }); | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from "./hooks"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| { | ||
| "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" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ | |
| "@blueprintjs/popover2": "^1.13.3", | ||
| "@headlessui/react": "^1.7.3", | ||
| "@popperjs/core": "^2.11.8", | ||
| "@plane/helpers": "*", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification Remove unused dependency The
Analysis chainVerify the usage and version specification of the new dependency. I noticed that a new dependency
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 executedThe 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 |
||
| "clsx": "^2.0.0", | ||
| "emoji-picker-react": "^4.5.16", | ||
| "lodash": "^4.17.21", | ||
|
|
||
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip
Codebase Verification
Old implementation of
useOutsideClickDetectorstill exists.The
useOutsideClickDetectorhook is being correctly imported from the@plane/helperspackage 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.tsxAnalysis chain
Verify the usage of the
@plane/helperspackage.To ensure that the new
@plane/helperspackage is being utilized correctly, please verify that:useOutsideClickDetectorhook have been updated to use the new package.Run the following script to verify the package usage:
Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 244
Script:
Length of output: 12689