Skip to content

Commit 415efc9

Browse files
amitsingh-007claude
andcommitted
refactor: fix first-paint viewport check and split colliding route constants
Three deliberate behaviour changes. 1. usePlatform -> useIsMobile, with a correct initial value. The hook was a viewport media query wearing the name of the platform seam. Mantine 9.5 defaults to getInitialValueInEffect: true and ends with `return matches || false`, so with no initial value the first render reported "not desktop" everywhere - including the 800px-wide extension popup. Every consumer painted a mobile layout for one frame: Bookmark mounted each row as an <a href> and then swapped it for plain text, and Persons computed a mobile column count and row height. The fix passes `true`, not `false`. Passing `false` would be a no-op, since `matches || false` already yields false today. Deliberately not using getInitialValueInEffect: false, which reads window.matchMedia during render and would risk a Next.js hydration mismatch. Trade-off: real mobile web (<768px) now gets a one-frame desktop paint instead. 2. ToggleExtension: one source of truth. The component kept a local extState whose getIsExtensionActive() was exactly the store's isExtensionActive, with dispatchActionAndSetState existing only to write both. useExtStore now exposes { isExtensionActive, setIsExtensionActive } instead of the always-paired turnOnExtension/turnOffExtension. The store default stays true on purpose: Authenticate reads it on mount and would auto-sign-out if it started false while storage resolves. Trade-off: the first-paint flash moves rather than disappearing. Today a stored-ACTIVE user sees unchecked->checked; now a stored-INACTIVE user sees checked->unchecked. Removing it entirely needs the store hydrated before first paint. 3. Split the colliding ROUTES. @bypass/shared exported HOMEPAGE: '/popup.html' while the web app exported its own ROUTES with HOMEPAGE: '/'. The web app imported both, three lines apart in two files, under the same identifier - so one wrong auto-import was a silent 404 or a broken RESTRICTED_PATHS check with no type error. Shared ROUTES now holds only the genuinely shared panel routes. The extension entry route moves to POPUP_HOMEPAGE in the extension, and the web constant is renamed WEB_ROUTES so the two cannot be confused. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 62ed560 commit 415efc9

13 files changed

Lines changed: 59 additions & 51 deletions

File tree

apps/extension/src/constants/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,6 @@ export const MAX_PANEL_SIZE = {
99
};
1010

1111
export const TEST_AUTH_DATA_KEY = '__test_auth_data';
12+
13+
/** Extension popup entry route. Not in @bypass/shared: meaningless on web. */
14+
export const POPUP_HOMEPAGE = '/popup.html';

apps/extension/src/entrypoints/popup/panels/HomePopup/components/ToggleExtension.tsx

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,30 @@
11
import { Switch } from '@bypass/ui';
2-
import { useCallback, useEffect, useState } from 'react';
2+
import { useEffect } from 'react';
33

44
import { EExtensionState } from '@/constants';
55
import { extStateItem } from '@/storage/items';
66
import { getIsExtensionActive } from '@/utils/common';
77
import useExtStore from '@store/extension';
88

99
function ToggleExtension() {
10-
const turnOnExtension = useExtStore((state) => state.turnOnExtension);
11-
const turnOffExtension = useExtStore((state) => state.turnOffExtension);
12-
const [extState, setExtState] = useState<EExtensionState>(
13-
EExtensionState.INACTIVE
14-
);
15-
16-
const dispatchActionAndSetState = useCallback(
17-
(_extState: EExtensionState, isActive: boolean) => {
18-
setExtState(_extState);
19-
const action = isActive ? turnOnExtension : turnOffExtension;
20-
action();
21-
},
22-
[turnOnExtension, turnOffExtension]
10+
const isActive = useExtStore((state) => state.isExtensionActive);
11+
const setIsExtensionActive = useExtStore(
12+
(state) => state.setIsExtensionActive
2313
);
2414

2515
useEffect(() => {
26-
extStateItem.getValue().then((_extState) => {
27-
const isActive = getIsExtensionActive(_extState);
28-
dispatchActionAndSetState(_extState, isActive);
16+
extStateItem.getValue().then((extState) => {
17+
setIsExtensionActive(getIsExtensionActive(extState));
2918
});
30-
}, [dispatchActionAndSetState]);
19+
}, [setIsExtensionActive]);
3120

3221
const handleToggle = (checked: boolean) => {
33-
const extensionState = checked
34-
? EExtensionState.ACTIVE
35-
: EExtensionState.INACTIVE;
36-
extStateItem.setValue(extensionState);
37-
dispatchActionAndSetState(extensionState, checked);
22+
extStateItem.setValue(
23+
checked ? EExtensionState.ACTIVE : EExtensionState.INACTIVE
24+
);
25+
setIsExtensionActive(checked);
3826
};
3927

40-
const isActive = getIsExtensionActive(extState);
4128
return (
4229
<div className="flex items-center gap-2">
4330
<Switch checked={isActive} onCheckedChange={handleToggle} />
Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import { ROUTES } from '@bypass/shared';
21
import { Route } from 'wouter';
32

3+
import { POPUP_HOMEPAGE } from '@/constants';
4+
45
import PopupHome from '../containers/PopupHome';
56

67
export const HomePageRoute = (
7-
<Route path={ROUTES.HOMEPAGE} component={PopupHome} />
8+
<Route path={POPUP_HOMEPAGE} component={PopupHome} />
89
);

apps/extension/src/store/extension.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@ import { create } from 'zustand';
22

33
interface State {
44
isExtensionActive: boolean;
5-
turnOnExtension: VoidFunction;
6-
turnOffExtension: VoidFunction;
5+
setIsExtensionActive: (isExtensionActive: boolean) => void;
76
}
87

98
const useExtStore = create<State>()((set) => ({
9+
// Defaults to true: Authenticate reads this on mount and would auto-sign-out
10+
// if it started false while storage is still resolving
1011
isExtensionActive: true,
11-
turnOnExtension: () => set(() => ({ isExtensionActive: true })),
12-
turnOffExtension: () => set(() => ({ isExtensionActive: false })),
12+
setIsExtensionActive: (isExtensionActive: boolean) =>
13+
set(() => ({ isExtensionActive })),
1314
}));
1415

1516
export default useExtStore;

apps/web/src/app/components/AppHeader.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@ import Image from 'next/image';
66
import { useRouter } from 'next/navigation';
77
import { useEffect, useState } from 'react';
88

9-
import { ROUTES } from '@app/constants/routes';
9+
import { WEB_ROUTES } from '@app/constants/routes';
1010

1111
function AppHeader() {
1212
const router = useRouter();
1313
const [clickCount, setClickCount] = useState(0);
1414

1515
useEffect(() => {
1616
if (clickCount === 5) {
17-
router.push(ROUTES.BYPASS_LINKS_WEB);
17+
router.push(WEB_ROUTES.BYPASS_LINKS_WEB);
1818
}
1919
}, [clickCount, router]);
2020

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
export const ROUTES = {
1+
/**
2+
* Web-app-only routes. Named distinctly from `ROUTES` in @bypass/shared so an
3+
* auto-import cannot silently swap one for the other.
4+
*/
5+
export const WEB_ROUTES = {
26
HOMEPAGE: '/',
37
BYPASS_LINKS_WEB: '/web-ext',
48
} as const;

apps/web/src/app/provider/AuthProvider.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
useState,
1010
} from 'react';
1111

12-
import { ROUTES } from '../constants/routes';
12+
import { WEB_ROUTES } from '../constants/routes';
1313
import { onAuthStateChange } from '../helpers/firebase/auth';
1414

1515
interface IAuthContext {
@@ -22,7 +22,7 @@ const AuthContext = createContext<IAuthContext>({
2222
isLoginIntialized: false,
2323
});
2424

25-
const RESTRICTED_PATHS = new Set([ROUTES.HOMEPAGE]);
25+
const RESTRICTED_PATHS = new Set([WEB_ROUTES.HOMEPAGE]);
2626

2727
export function AuthProvider({ children }: PropsWithChildren) {
2828
const pathname = usePathname();

packages/shared/src/components/Bookmarks/components/Bookmark.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
TooltipTrigger,
88
} from '@bypass/ui';
99

10-
import usePlatform from '../../../hooks/usePlatform';
10+
import useIsMobile from '../../../hooks/useIsMobile';
1111
import useTaggedPersons from '../../Persons/hooks/useTaggedPersons';
1212
import Favicon from './Favicon';
1313
import PersonAvatars from './PersonAvatars';
@@ -36,7 +36,7 @@ function Bookmark({
3636
getFaviconUrl,
3737
}: BookmarkProps) {
3838
const { data: personsWithImageUrls = [] } = useTaggedPersons(taggedPersons);
39-
const isMobile = usePlatform();
39+
const isMobile = useIsMobile();
4040

4141
const handleOpenLink: React.MouseEventHandler<HTMLDivElement> = (event) => {
4242
if (event.ctrlKey || event.metaKey) {

packages/shared/src/components/Persons/components/Persons.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { useElementSize } from '@mantine/hooks';
33
import { useVirtualizer } from '@tanstack/react-virtual';
44
import { type ReactNode, use, useCallback, useState } from 'react';
55

6-
import usePlatform from '../../../hooks/usePlatform';
6+
import useIsMobile from '../../../hooks/useIsMobile';
77
import DynamicContext from '../../../provider/DynamicContext';
88
import { deserializeQueryStringToObject } from '../../../utils/url';
99
import { ScrollButton } from '../../ScrollButton';
@@ -42,7 +42,7 @@ function PersonsInner({
4242
personToOpenImage,
4343
renderPerson,
4444
}: InnerProps) {
45-
const isMobile = usePlatform();
45+
const isMobile = useIsMobile();
4646
const columnCount = getColumnCount(isMobile);
4747
const rowCount = Math.ceil(persons.length / columnCount);
4848
const columnDimension = (bodyWidth - 12) / columnCount; // Adjust scrollbar width

packages/shared/src/constants/routes.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1+
/**
2+
* Panel routes shared by the extension popup and the web app. The extension's
3+
* own entry route lives in the extension (POPUP_HOMEPAGE) since '/popup.html'
4+
* is meaningless on web.
5+
*/
16
export const ROUTES = {
2-
HOMEPAGE: '/popup.html',
37
SHORTCUTS_PANEL: '/shortcuts-panel/',
48
BOOKMARK_PANEL: '/bookmark-panel/',
59
PERSONS_PANEL: '/persons-panel/',

0 commit comments

Comments
 (0)