refactor: consolidate viewport hooks and remove defineProperty matchMedia stub - #28460
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
ibetitsmike
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
jeremyruppel
left a comment
jeremyruppel
left a comment
There was a problem hiding this comment.
LGTM! thanks for these fixups!
…terns and review skill
c1b0c82 to
021243d
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Follow-up to review feedback on #28387.
That PR added three hooks for one feature (
useIsBelowLgViewport,useIsBelowMdViewport, and the local single-useuseRightPanelNarrowSuppression) and stubbedwindow.matchMediain tests withObject.defineProperty.useMediaQuery(query). Callers pass the shared Tailwind-aligned query constants fromutils/mobile.ts. The one new hook replaces the two deleted ones (net -1).useRightPanelNarrowSuppressionintoAgentChatPage, its only consumer, and drop itsrenderHookunit suite. The behavior stays covered by the narrow-viewport stories; the widening-restore case moved into theNarrowingSuppressesExpandedPanelplay function.testHelpers/matchMedia.tsto install the stub withspyOnfromstorybook/testinstead ofObject.defineProperty. The helper is story-only now (stories run in real Chromium; jsdom has nomatchMedia), so any future unit test needing a stub should usevi.stubGlobaldirectly..claude/docs/FRONTEND_PATTERNS.mdnow bans new React hooks when an existing hook, a plain function, or component state suffices (FE3) and bans replacing browser globals withObject.definePropertyin tests or stories (FE9:vi.stubGlobal/spyOn), and notes thatrenderHooksuites for stateful UI hooks belong in the consuming component's story (FE1). Thefrontend-reviewskill checklist flags all three.site/AGENTS.mdis unchanged since it already defers to the patterns doc.Validation:
pnpm check,pnpm format:check,pnpm lint(biome, types, knip, circular deps, compiler check),AgentChatPage.test.ts(70 passed), story runs forAgentChatPage.stories.tsxandWorkspacePill.stories.tsx(49 passed in Chromium).