refactor(ui): migrate devtools panel to shadcn-vue#12
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d622760041
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72e3b8f541
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| :aria-label="isDark ? 'Switch to light theme' : 'Switch to dark theme'" |
There was a problem hiding this comment.
When the panel language is switched to zh_CN, this newly added theme button remains announced in English because the label is hard-coded instead of going through messages.json/t(). The repository instructions require updating both locales for user-visible strings; the same applies to the new Clear filter and Back to top labels, so screen-reader users still get mixed-language controls after toggling locales.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d9d43525b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| import assert from 'node:assert/strict' | ||
| import { describe, it } from 'node:test' |
There was a problem hiding this comment.
These new .node-test.ts files sit under entrypoints, which the WXT-generated tsconfig type-checks as part of pnpm compile; the project also narrows compilerOptions.types to chrome and does not add @types/node. As a result vue-tsc --noEmit will include this import and fail on missing node:assert/strict/node:test typings before the extension can compile. Please either exclude these tests from the app tsconfig or add a separate test tsconfig/Node typings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cfc15d901
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| if (!row.isDiff || valueType === 'left') { | ||
| return 'muted' |
There was a problem hiding this comment.
When a CSS custom property exists only on the right element, compareStyles emits left: '未定义', but this early return mutes every left cell before checking whether that cell is actually missing. That makes the missing side look like an ordinary value, while the reverse selection order gets the dashed missing treatment; please check for UNDEFINED_STYLE_VALUE on both sides so missing properties are highlighted regardless of selection order.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc6f291433
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| cssDiffs.length = 0 | ||
|
|
||
| // Send selected data to other windows/tabs | ||
| SM.send(selectedEl) |
There was a problem hiding this comment.
When a user removes just one selected element, this sends a one-element payload to the other synced DevTools panels. The receiver clears and repopulates selectedEl, but only clears cssDiffs for an empty payload or recomputes for two elements, so synced panels keep showing the stale diff table while displaying only one selected element. This only happens in the cross-window/tab sync path for the new single-selection remove action; clear or recompute the remote diffs for payloads with fewer than two selections.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbbf92b9fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| body { | ||
| @apply bg-background text-foreground antialiased; | ||
| min-width: 520px; | ||
| padding: 16px; | ||
| overflow-x: hidden; | ||
| overflow-y: hidden; |
There was a problem hiding this comment.
Because browsers give body an 8px default margin, the new fixed-height panel (100vh - 32px) plus 16px top/bottom padding still occupies more than the DevTools pane once overflow-y is hidden here. In normal browser styling this clips the bottom of the sidebar/table instead of allowing it to scroll; reset the body margin or account for it before disabling overflow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f32c9147b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| min-width: 520px; | ||
| padding: 16px; | ||
| overflow-x: hidden; |
There was a problem hiding this comment.
When the Elements sidebar is resized below 520px, this new body min-width forces the whole panel wider than the available DevTools pane, and overflow-x: hidden prevents users from scrolling to the clipped header controls/content. Since the diff table already has its own horizontal scroller, the page shell should not impose an unscrollable minimum width on the sidebar.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cd1a71136
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| @@ -0,0 +1,38 @@ | |||
| import assert from 'node:assert/strict' | |||
| import { describe, it } from 'node:test' | |||
There was a problem hiding this comment.
These new node:test files are never exercised by the repository's test command: package.json still runs vitest run, and vitest.config.ts limits discovery to tests/unit/**/*.test.ts, so files named entrypoints/**/.node-test.ts are silently skipped. That means regressions in getDiffValueTone/getDiffValueClass can pass CI unless these cases are moved under tests/unit with the configured *.test.ts pattern or a Node test script is added.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccd898a735
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| variant="ghost" | ||
| size="icon" | ||
| data-testid="theme-toggle" | ||
| :aria-label="isDark ? 'Switch to light theme' : 'Switch to dark theme'" |
There was a problem hiding this comment.
When the panel locale is switched to Chinese, screen-reader users still get English for this newly added theme toggle label because the strings are hard-coded instead of going through t(...) and the locale files. This also violates the repo's i18n convention to update both locale files for user-visible strings; please add message keys for these new control labels and use them here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a47bf4cef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| min-width: 0; | ||
| margin: 0; | ||
| padding: 16px; | ||
| overflow-y: hidden; |
There was a problem hiding this comment.
When the DevTools pane is docked/resized short enough that the header plus selection/filter cards exceed the available height, the panel content overflows the fixed-height <main> but the body is no longer scrollable here; only the diff table container has overflow-auto, so the lower controls/table can be clipped and unreachable. Please keep a page-level vertical scroll fallback or make the whole panel shell scroll in this constrained-height case.
Useful? React with 👍 / 👎.
1 participant
Summary
entrypoints/**/*.node-test.tsinto the configured Vitest unit runner sopnpm test:unitexercises locale/diff/theme helpers.main, update WXT/Tailwind-related dependency state, add the missing ESLint node import resolver, and refreshimg/screenshot.pngfor the new UI.Validation
pnpm exec eslint .pnpm run compilepnpm test:unitpnpm test:e2e