[WEB-5501] refactor: optimize component structures and improve hooks (#8174)

* [WEB-5501] refactor: optimize component structures and improve hooks

- Updated type definitions in AppProvider to use React.ReactNode for children.
- Enhanced HomePeekOverviewsRoot by using MobX observer and integrating issue detail hook.
- Optimized ContentOverflowWrapper to prevent unnecessary re-renders by adjusting useEffect dependencies.
- Updated DashboardQuickLinks to include necessary dependencies in useCallback.
- Refactored GlobalShortcutsProvider to utilize refs for context and handler management, improving performance.
- Changed useCurrentTime to update every minute instead of every second.
- Refactored outside click hooks to use useCallback for better performance.
- Improved IntercomProvider and PostHogProvider to prevent multiple initializations using refs.

* refactor: simplify conditional rendering in HomePeekOverviewsRoot component

* refactor: improve outside click detection in sidebar and peek overview hooks

* refactor: enhance IntercomProvider and PostHogProvider with hydration state management
This commit is contained in:
Prateek Shourya 2025-11-25 18:52:20 +05:30 committed by GitHub
parent 31e8563725
commit 3436c4f1f5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 155 additions and 119 deletions

View file

@ -1,4 +1,3 @@
import type { FC, ReactNode } from "react";
import { lazy, Suspense } from "react"; import { lazy, Suspense } from "react";
import { useTheme, ThemeProvider } from "next-themes"; import { useTheme, ThemeProvider } from "next-themes";
import { SWRConfig } from "swr"; import { SWRConfig } from "swr";
@ -31,7 +30,7 @@ const IntercomProvider = lazy(function IntercomProvider() {
}); });
export interface IAppProvider { export interface IAppProvider {
children: ReactNode; children: React.ReactNode;
} }
function ToastWithTheme() { function ToastWithTheme() {

View file

@ -1,9 +1,9 @@
import { observer } from "mobx-react";
import { IssuePeekOverview } from "@/components/issues/peek-overview"; import { IssuePeekOverview } from "@/components/issues/peek-overview";
import { useIssueDetail } from "@/hooks/store/use-issue-detail";
export function HomePeekOverviewsRoot() { export const HomePeekOverviewsRoot = observer(function HomePeekOverviewsRoot() {
return ( const { peekIssue } = useIssueDetail();
<>
<IssuePeekOverview /> return peekIssue ? <IssuePeekOverview /> : null;
</> });
);
}

View file

@ -77,19 +77,20 @@ export const ContentOverflowWrapper = observer(function ContentOverflowWrapper(p
resizeObserver.disconnect(); resizeObserver.disconnect();
mutationObserver.disconnect(); mutationObserver.disconnect();
}; };
}, [contentRef?.current]); }, []);
useEffect(() => { useEffect(() => {
if (!containerRef.current) return; const container = containerRef.current;
if (!container) return;
const handleTransitionEnd = () => { const handleTransitionEnd = () => {
setIsTransitioning(false); setIsTransitioning(false);
}; };
containerRef.current.addEventListener("transitionend", handleTransitionEnd); container.addEventListener("transitionend", handleTransitionEnd);
return () => { return () => {
containerRef.current?.removeEventListener("transitionend", handleTransitionEnd); container.removeEventListener("transitionend", handleTransitionEnd);
}; };
}, []); }, []);

View file

@ -20,7 +20,7 @@ export const DashboardQuickLinks = observer(function DashboardQuickLinks(props:
const handleCreateLinkModal = useCallback(() => { const handleCreateLinkModal = useCallback(() => {
toggleLinkModal(true); toggleLinkModal(true);
setLinkData(undefined); setLinkData(undefined);
}, []); }, [toggleLinkModal, setLinkData]);
useSWR( useSWR(
workspaceSlug ? `HOME_LINKS_${workspaceSlug}` : null, workspaceSlug ? `HOME_LINKS_${workspaceSlug}` : null,

View file

@ -1,9 +1,8 @@
import { useEffect } from "react"; import { useEffect, useRef } from "react";
import { observer } from "mobx-react"; import { observer } from "mobx-react";
import { useParams } from "next/navigation"; import { useParams } from "next/navigation";
// hooks // hooks
import { usePowerK } from "@/hooks/store/use-power-k"; import { usePowerK } from "@/hooks/store/use-power-k";
import { useAppRouter } from "@/hooks/use-app-router";
// local imports // local imports
import { detectContextFromURL } from "./core/context-detector"; import { detectContextFromURL } from "./core/context-detector";
import { ShortcutHandler } from "./core/shortcut-handler"; import { ShortcutHandler } from "./core/shortcut-handler";
@ -22,7 +21,6 @@ type GlobalShortcutsProps = {
export const GlobalShortcutsProvider = observer(function GlobalShortcutsProvider(props: GlobalShortcutsProps) { export const GlobalShortcutsProvider = observer(function GlobalShortcutsProvider(props: GlobalShortcutsProps) {
const { context, commands } = props; const { context, commands } = props;
// router // router
const router = useAppRouter();
const params = useParams(); const params = useParams();
// store hooks // store hooks
const { commandRegistry, isShortcutsListModalOpen, setActiveContext, togglePowerKModal, toggleShortcutsListModal } = const { commandRegistry, isShortcutsListModalOpen, setActiveContext, togglePowerKModal, toggleShortcutsListModal } =
@ -40,21 +38,40 @@ export const GlobalShortcutsProvider = observer(function GlobalShortcutsProvider
commandRegistry.registerMultiple(commands); commandRegistry.registerMultiple(commands);
}, [commandRegistry, commands]); }, [commandRegistry, commands]);
// Setup global shortcut handler // Store context in ref to avoid recreation on context changes
const contextRef = useRef(context);
useEffect(() => { useEffect(() => {
const handler = new ShortcutHandler( contextRef.current = context;
}, [context]);
// Store handler in ref to avoid recreation on context changes
const handlerRef = useRef<ShortcutHandler | null>(null);
// Setup global shortcut handler - only recreate when commandRegistry or togglePowerKModal changes
useEffect(() => {
// Clean up previous handler if it exists
if (handlerRef.current) {
document.removeEventListener("keydown", handlerRef.current.handleKeyDown);
handlerRef.current.destroy();
}
// Create new handler with function that reads from ref
handlerRef.current = new ShortcutHandler(
commandRegistry, commandRegistry,
() => context, () => contextRef.current,
() => togglePowerKModal(true) () => togglePowerKModal(true)
); );
document.addEventListener("keydown", handler.handleKeyDown); document.addEventListener("keydown", handlerRef.current.handleKeyDown);
return () => { return () => {
document.removeEventListener("keydown", handler.handleKeyDown); if (handlerRef.current) {
handler.destroy(); document.removeEventListener("keydown", handlerRef.current.handleKeyDown);
handlerRef.current.destroy();
handlerRef.current = null;
}
}; };
}, [context, router, commandRegistry, togglePowerKModal]); }, [commandRegistry, togglePowerKModal]);
return <ShortcutsModal isOpen={isShortcutsListModalOpen} onClose={() => toggleShortcutsListModal(false)} />; return <ShortcutsModal isOpen={isShortcutsListModalOpen} onClose={() => toggleShortcutsListModal(false)} />;
}); });

View file

@ -2,11 +2,11 @@ import { useEffect, useState } from "react";
export const useCurrentTime = () => { export const useCurrentTime = () => {
const [currentTime, setCurrentTime] = useState(new Date()); const [currentTime, setCurrentTime] = useState(new Date());
// update the current time every second // update the current time every minute (60000ms)
useEffect(() => { useEffect(() => {
const intervalId = setInterval(() => { const intervalId = setInterval(() => {
setCurrentTime(new Date()); setCurrentTime(new Date());
}, 1000); }, 60000);
return () => clearInterval(intervalId); return () => clearInterval(intervalId);
}, []); }, []);

View file

@ -1,42 +1,44 @@
import type React from "react"; import type React from "react";
import { useEffect } from "react"; import { useEffect, useCallback } from "react";
const useExtendedSidebarOutsideClickDetector = ( const useExtendedSidebarOutsideClickDetector = (
ref: React.RefObject<HTMLElement>, ref: React.RefObject<HTMLElement>,
callback: () => void, callback: () => void,
targetId: string targetId: string
) => { ) => {
const handleClick = (event: MouseEvent) => { const handleClick = useCallback(
if (ref.current && !ref.current.contains(event.target as Node)) { (event: MouseEvent) => {
// check for the closest element with attribute name data-prevent-outside-click if (!(event.target instanceof HTMLElement)) return;
const preventOutsideClickElement = (event.target as HTMLElement | undefined)?.closest( if (ref.current && !ref.current.contains(event.target)) {
"[data-prevent-outside-click]" // check for the closest element with attribute name data-prevent-outside-click
); const preventOutsideClickElement = event.target.closest("[data-prevent-outside-click]");
// if the closest element with attribute name data-prevent-outside-click is found, return // if the closest element with attribute name data-prevent-outside-click is found, return
if (preventOutsideClickElement) { if (preventOutsideClickElement) {
return;
}
// check if the click target is the current issue element or its children
let targetElement = event.target as HTMLElement | null;
while (targetElement) {
if (targetElement.id === targetId) {
// if the click target is the current issue element, return
return; return;
} }
targetElement = targetElement.parentElement; // check if the click target is the current issue element or its children
let targetElement: HTMLElement | null = event.target;
while (targetElement) {
if (targetElement.id === targetId) {
// if the click target is the current issue element, return
return;
}
targetElement = targetElement.parentElement;
}
const delayOutsideClickElement = event.target.closest("[data-delay-outside-click]");
if (delayOutsideClickElement) {
// if the click target is the closest element with attribute name data-delay-outside-click, delay the callback
setTimeout(() => {
callback();
}, 0);
return;
}
// else, call the callback immediately
callback();
} }
const delayOutsideClickElement = (event.target as HTMLElement | undefined)?.closest("[data-delay-outside-click]"); },
if (delayOutsideClickElement) { [ref, callback, targetId]
// if the click target is the closest element with attribute name data-delay-outside-click, delay the callback );
setTimeout(() => {
callback();
}, 0);
return;
}
// else, call the callback immediately
callback();
}
};
useEffect(() => { useEffect(() => {
document.addEventListener("mousedown", handleClick); document.addEventListener("mousedown", handleClick);
@ -44,7 +46,7 @@ const useExtendedSidebarOutsideClickDetector = (
return () => { return () => {
document.removeEventListener("mousedown", handleClick); document.removeEventListener("mousedown", handleClick);
}; };
}, []); }, [handleClick]);
}; };
export default useExtendedSidebarOutsideClickDetector; export default useExtendedSidebarOutsideClickDetector;

View file

@ -1,42 +1,44 @@
import type React from "react"; import type React from "react";
import { useEffect } from "react"; import { useEffect, useCallback } from "react";
const usePeekOverviewOutsideClickDetector = ( const usePeekOverviewOutsideClickDetector = (
ref: React.RefObject<HTMLElement>, ref: React.RefObject<HTMLElement>,
callback: () => void, callback: () => void,
issueId: string issueId: string
) => { ) => {
const handleClick = (event: MouseEvent) => { const handleClick = useCallback(
if (ref.current && !ref.current.contains(event.target as Node)) { (event: MouseEvent) => {
// check for the closest element with attribute name data-prevent-outside-click if (!(event.target instanceof HTMLElement)) return;
const preventOutsideClickElement = (event.target as HTMLElement | undefined)?.closest( if (ref.current && !ref.current.contains(event.target)) {
"[data-prevent-outside-click]" // check for the closest element with attribute name data-prevent-outside-click
); const preventOutsideClickElement = event.target.closest("[data-prevent-outside-click]");
// if the closest element with attribute name data-prevent-outside-click is found, return // if the closest element with attribute name data-prevent-outside-click is found, return
if (preventOutsideClickElement) { if (preventOutsideClickElement) {
return;
}
// check if the click target is the current issue element or its children
let targetElement = event.target as HTMLElement | null;
while (targetElement) {
if (targetElement.id === `issue-${issueId}`) {
// if the click target is the current issue element, return
return; return;
} }
targetElement = targetElement.parentElement; // check if the click target is the current issue element or its children
let targetElement: HTMLElement | null = event.target;
while (targetElement) {
if (targetElement.id === `issue-${issueId}`) {
// if the click target is the current issue element, return
return;
}
targetElement = targetElement.parentElement;
}
const delayOutsideClickElement = event.target.closest("[data-delay-outside-click]");
if (delayOutsideClickElement) {
// if the click target is the closest element with attribute name data-delay-outside-click, delay the callback
setTimeout(() => {
callback();
}, 0);
return;
}
// else, call the callback immediately
callback();
} }
const delayOutsideClickElement = (event.target as HTMLElement | undefined)?.closest("[data-delay-outside-click]"); },
if (delayOutsideClickElement) { [ref, callback, issueId]
// if the click target is the closest element with attribute name data-delay-outside-click, delay the callback );
setTimeout(() => {
callback();
}, 0);
return;
}
// else, call the callback immediately
callback();
}
};
useEffect(() => { useEffect(() => {
document.addEventListener("mousedown", handleClick); document.addEventListener("mousedown", handleClick);
@ -44,7 +46,7 @@ const usePeekOverviewOutsideClickDetector = (
return () => { return () => {
document.removeEventListener("mousedown", handleClick); document.removeEventListener("mousedown", handleClick);
}; };
}); }, [handleClick]);
}; };
export default usePeekOverviewOutsideClickDetector; export default usePeekOverviewOutsideClickDetector;

View file

@ -1,4 +1,4 @@
import React, { useEffect } from "react"; import React, { useEffect, useRef, useState } from "react";
import { Intercom, show, hide, onHide } from "@intercom/messenger-js-sdk"; import { Intercom, show, hide, onHide } from "@intercom/messenger-js-sdk";
import { observer } from "mobx-react"; import { observer } from "mobx-react";
// store hooks // store hooks
@ -16,27 +16,38 @@ const IntercomProvider = observer(function IntercomProvider(props: IntercomProvi
const { data: user } = useUser(); const { data: user } = useUser();
const { config } = useInstance(); const { config } = useInstance();
const { isIntercomToggle, toggleIntercom } = useTransient(); const { isIntercomToggle, toggleIntercom } = useTransient();
// refs
const isInitializedRef = useRef(false);
// states
const [hydrated, setHydrated] = useState(false);
// derived values
const isIntercomEnabled = user && config && config.is_intercom_enabled && config.intercom_app_id;
useEffect(() => { useEffect(() => {
if (!hydrated) return;
if (isIntercomToggle) show(); if (isIntercomToggle) show();
else hide(); else hide();
}, [isIntercomToggle]); }, [hydrated, isIntercomToggle]);
onHide(() => {
toggleIntercom(false);
});
useEffect(() => { useEffect(() => {
if (user && config?.is_intercom_enabled && config.intercom_app_id) { if (!hydrated) return;
Intercom({ onHide(() => {
app_id: config.intercom_app_id || "", toggleIntercom(false);
user_id: user.id, });
name: `${user.first_name} ${user.last_name}`, }, [hydrated, toggleIntercom]);
email: user.email,
hide_default_launcher: true, useEffect(() => {
}); if (!isIntercomEnabled || isInitializedRef.current) return; // prevent multiple initializations
} Intercom({
}, [user, config, toggleIntercom]); app_id: config.intercom_app_id || "",
user_id: user.id,
name: `${user.first_name} ${user.last_name}`,
email: user.email,
hide_default_launcher: true,
});
isInitializedRef.current = true;
setHydrated(true);
}, [isIntercomEnabled, config, user]);
return <>{children}</>; return <>{children}</>;
}); });

View file

@ -1,5 +1,5 @@
import type { ReactNode } from "react"; import type { ReactNode } from "react";
import { lazy, Suspense, useEffect, useState } from "react"; import { lazy, Suspense, useEffect, useCallback, useRef, useState } from "react";
import { PostHogProvider as PHProvider } from "@posthog/react"; import { PostHogProvider as PHProvider } from "@posthog/react";
import { observer } from "mobx-react"; import { observer } from "mobx-react";
import { useParams } from "next/navigation"; import { useParams } from "next/navigation";
@ -29,6 +29,8 @@ const PostHogProvider = observer(function PostHogProvider(props: IPosthogWrapper
const { instance } = useInstance(); const { instance } = useInstance();
const { workspaceSlug, projectId } = useParams(); const { workspaceSlug, projectId } = useParams();
const { getWorkspaceRoleByWorkspaceSlug, getProjectRoleByWorkspaceSlugAndProjectId } = useUserPermissions(); const { getWorkspaceRoleByWorkspaceSlug, getProjectRoleByWorkspaceSlugAndProjectId } = useUserPermissions();
// refs
const isInitializedRef = useRef(false);
// states // states
const [hydrated, setHydrated] = useState(false); const [hydrated, setHydrated] = useState(false);
// derived values // derived values
@ -61,10 +63,11 @@ const PostHogProvider = observer(function PostHogProvider(props: IPosthogWrapper
}, [user, currentProjectRole, currentWorkspaceRole, currentWorkspace, hydrated]); }, [user, currentProjectRole, currentWorkspaceRole, currentWorkspace, hydrated]);
useEffect(() => { useEffect(() => {
if (isInitializedRef.current) return; // prevent multiple initializations
const posthogKey = process.env.VITE_POSTHOG_KEY; const posthogKey = process.env.VITE_POSTHOG_KEY;
const posthogHost = process.env.VITE_POSTHOG_HOST; const posthogHost = process.env.VITE_POSTHOG_HOST;
const isDebugMode = process.env.VITE_POSTHOG_DEBUG === "1"; const isDebugMode = process.env.VITE_POSTHOG_DEBUG === "1";
if (posthogKey && posthogHost) { if (posthogKey && posthogHost && !posthog.__loaded) {
posthog.init(posthogKey, { posthog.init(posthogKey, {
api_host: posthogHost, api_host: posthogHost,
ui_host: posthogHost, ui_host: posthogHost,
@ -74,31 +77,32 @@ const PostHogProvider = observer(function PostHogProvider(props: IPosthogWrapper
capture_pageleave: true, capture_pageleave: true,
disable_session_recording: true, disable_session_recording: true,
}); });
isInitializedRef.current = true;
setHydrated(true); setHydrated(true);
} }
}, []); }, []);
useEffect(() => { const clickHandler = useCallback((event: MouseEvent) => {
const clickHandler = (event: MouseEvent) => { const target = event.target as HTMLElement;
const target = event.target as HTMLElement; // Use closest to find the nearest parent element with data-ph-element attribute
// Use closest to find the nearest parent element with data-ph-element attribute const elementWithAttribute = target.closest("[data-ph-element]") as HTMLElement;
const elementWithAttribute = target.closest("[data-ph-element]") as HTMLElement; if (elementWithAttribute) {
if (elementWithAttribute) { const element = elementWithAttribute.getAttribute("data-ph-element");
const element = elementWithAttribute.getAttribute("data-ph-element"); if (element) {
if (element) { captureClick({ elementName: element });
captureClick({ elementName: element });
}
} }
};
if (is_posthog_enabled && hydrated) {
document.addEventListener("click", clickHandler);
} }
}, []);
useEffect(() => {
if (!is_posthog_enabled || !hydrated) return;
document.addEventListener("click", clickHandler);
return () => { return () => {
document.removeEventListener("click", clickHandler); document.removeEventListener("click", clickHandler);
}; };
}, [hydrated, is_posthog_enabled]); }, [hydrated, is_posthog_enabled, clickHandler]);
if (is_posthog_enabled && hydrated) if (is_posthog_enabled && hydrated)
return ( return (