This repository was archived by the owner on Jun 27, 2026. It is now read-only.
Migrate HeroUI to v3 - #215
Open
hiroto7 wants to merge 1 commit into
Open
Conversation
WalkthroughHeroUI v2からv3への移行を行い、依存関係とスタイルの取り込み方法を更新。複合コンポーネントAPI（Tabs/ProgressBar/Accordion/ListBox/Alert等）へ移行し、HeroUIプロバイダーとプラグインを削除、複数コンポーネントのイベント/ナビゲーション周りを再構成しました。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/FeatureList.tsx (1)
284-288:⚠️ Potential issue | 🟠 MajorIterator Helpers への依存を避けてください。
Line 287 の
keys.values().map(String).toArray()は Iterator Helpers を使用していますが、プロジェクトのtsconfig.app.jsonではtarget: "ES2020"に設定されており、Iterator Helpers は ES2020 標準に含まれていません。このコードはコンパイルまたは実行時に失敗する可能性があります。標準的なArray.from(keys, String)を使用してください。修正案
onExpandedChange={(keys: Set<Key>) => { if (!(keys instanceof Set)) return; void set({ - selectedFeatureListSections: keys.values().map(String).toArray(), + selectedFeatureListSections: Array.from(keys, String), }); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/FeatureList.tsx` around lines 284 - 288, In the onExpandedChange callback (the handler using the keys: Set<Key> parameter) avoid Iterator Helpers by replacing keys.values().map(String).toArray() with a standard Array.from conversion; update the call that sets selectedFeatureListSections (the set({...}) invocation) to use Array.from(keys, String) so it works with ES2020 and preserves the same stringified values.
🧹 Nitpick comments (1)
src/CustomAlert.tsx (1)
18-38:Alert.Titleの内側の折りたたみトグルボタンを外へ移動してください。HeroUI v3 の公式ドキュメントによると、Alert の推奨アナトミーは以下の通りです：
Alert→Alert.Indicator（オプション） +Alert.Content（Alert.TitleとAlert.Descriptionを含む） + アクションボタン等（Alert.Contentの兄弟として配置）。現在の実装では、インタラクティブな
ButtonがAlert.Titleの内側にネストされており、見出し要素の中にボタンがある形となっているため、スクリーンリーダーでの読み上げが不自然になり、セマンティクスとして不適切です。タイトルテキストとトグルボタンを分離し、ボタンをAlert直下（Alert.Contentの兄弟）に配置する構成に変更をおすすめします。♻️ 提案する構成例
- <Alert status={status} className="flex flex-col items-stretch gap-2"> - <Alert.Content className="w-full"> - <Alert.Title className="flex items-center justify-between gap-x-1"> - {title} - {isCollapsible && ( - <Button - size="sm" - variant="tertiary" - isIconOnly - onPress={() => setIsExpanded(!isExpanded)} - > - {isExpanded ? <ChevronUpIcon /> : <ChevronDownIcon />} - </Button> - )} - </Alert.Title> - {shouldShowDetail && description && ( - <Alert.Description className="pl-0">{description}</Alert.Description> - )} - </Alert.Content> - {shouldShowDetail && endContent} - </Alert> + <Alert status={status} className="flex flex-col items-stretch gap-2"> + <div className="flex w-full items-start justify-between gap-x-1"> + <Alert.Content className="w-full"> + <Alert.Title>{title}</Alert.Title> + {shouldShowDetail && description && ( + <Alert.Description className="pl-0">{description}</Alert.Description> + )} + </Alert.Content> + {isCollapsible && ( + <Button + size="sm" + variant="tertiary" + isIconOnly + onPress={() => setIsExpanded(!isExpanded)} + > + {isExpanded ? <ChevronUpIcon /> : <ChevronDownIcon />} + </Button> + )} + </div> + {shouldShowDetail && endContent} + </Alert>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/CustomAlert.tsx` around lines 18 - 38, The toggle Button is nested inside Alert.Title which breaks the recommended Alert anatomy; move the collapsible toggle out of Alert.Title and render it as a sibling of Alert.Content directly under Alert (when isCollapsible is true), keeping the existing props (size, variant, isIconOnly, onPress using setIsExpanded, and icons using isExpanded) and preserving layout (wrap Alert.Content and the Button in a flex container or use CSS classes to achieve the previous spacing) so title text remains plain inside Alert.Title and the interactive Button becomes an Alert sibling alongside Alert.Content (ensure shouldShowDetail/endContent logic still renders correctly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/contexts/ChromeStorageContext.tsx`:
- Around line 45-64: The current useEffect blocks call getItems() and swallow
rejections (using void), which can leave items undefined and the UI permanently
blank; update both places that call getItems() (the initial loader in the first
useEffect and the onChanged listener in the second useEffect) to attach a .catch
handler that logs the error (e.g., console.error or processLogger.error) and
optionally set a safe fallback via setItems or leave previous state; ensure the
listener still calls setItems on success and that cleanup (removing the
onChanged listener) remains unchanged.
In `@src/GroupProjectList.tsx`:
- Around line 72-87: loadingItem currently uses duplicate key/id "skeleton"
causing collisions when both Groups and Projects render loading items; update
the ListBox.Item instances (the loadingItem usage and any repeats around lines
with ListBox.Item) to use section-unique identifiers (e.g., id/key like
"skeleton-groups" and "skeleton-projects" or include the section name) so each
loading item has a unique key/id; ensure you change both key and id attributes
for the ListBox.Item(s) that render loading placeholders (the loadingItem
variable/usages).
- Around line 150-164: The icon-only Button rendering StarredIcon/StarIcon lacks
an accessible name; update the Button (the instance using props isIconOnly,
variant="tertiary", size="sm", and onPress={() => onStar(!starred)}) to include
an aria-label that reflects the current state (e.g., "Unstar" when starred is
true, "Star" when starred is false), so assistive technologies can announce the
action; ensure the label changes with the starred boolean and is present
alongside the existing props.
In `@src/options/App.tsx`:
- Around line 57-70: Remove the redundant id on the Checkbox root and the
htmlFor on the Label inside Checkbox.Content: in the Checkbox component where
autoTabSwitch is used (the Checkbox with id="auto-tab-switch"), delete the id
prop and remove the htmlFor="auto-tab-switch" from the Label inside
Checkbox.Content so the Label relies on HeroUI/React Aria context for
association (match how Radio.Content is implemented elsewhere).
---
Outside diff comments:
In `@src/FeatureList.tsx`:
- Around line 284-288: In the onExpandedChange callback (the handler using the
keys: Set<Key> parameter) avoid Iterator Helpers by replacing
keys.values().map(String).toArray() with a standard Array.from conversion;
update the call that sets selectedFeatureListSections (the set({...})
invocation) to use Array.from(keys, String) so it works with ES2020 and
preserves the same stringified values.
---
Nitpick comments:
In `@src/CustomAlert.tsx`:
- Around line 18-38: The toggle Button is nested inside Alert.Title which breaks
the recommended Alert anatomy; move the collapsible toggle out of Alert.Title
and render it as a sibling of Alert.Content directly under Alert (when
isCollapsible is true), keeping the existing props (size, variant, isIconOnly,
onPress using setIsExpanded, and icons using isExpanded) and preserving layout
(wrap Alert.Content and the Button in a flex container or use CSS classes to
achieve the previous spacing) so title text remains plain inside Alert.Title and
the interactive Button becomes an Alert sibling alongside Alert.Content (ensure
shouldShowDetail/endContent logic still renders correctly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cdf5f457-6c87-4863-8a44-2d18498a2e35
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
package.jsonsrc/App.tsxsrc/CustomAlert.tsxsrc/FeatureList.tsxsrc/GroupProjectList.tsxsrc/contexts/ChromeStorageContext.tsxsrc/hero.tssrc/index.csssrc/main.tsxsrc/options/App.tsxsrc/options/index.csssrc/options/main.tsx
💤 Files with no reviewable changes (1)
- src/hero.ts
Comment on lines
+45
to
+64
| useEffect(() => { | ||
| let isMounted = true; | ||
| void getItems().then((items) => { | ||
| if (isMounted) setItems(items as T); | ||
| }); | ||
| return () => { | ||
| isMounted = false; | ||
| }; | ||
| }, [getItems]); | ||
|
|
||
| useEffect(() => { | ||
| if (!watch) return; | ||
| chrome.storage[area].onChanged.addListener(() => void load()); | ||
| const listener = () => { | ||
| void getItems().then((items) => setItems(items as T)); | ||
| }; | ||
| chrome.storage[area].onChanged.addListener(listener); | ||
| return () => { | ||
| chrome.storage[area].onChanged.removeListener(() => void load()); | ||
| chrome.storage[area].onChanged.removeListener(listener); | ||
| }; | ||
| }, [load]); | ||
| }, [getItems]); |
There was a problem hiding this comment.
getItems() の reject を捕捉していないため、ストレージ取得失敗時に UI が永久に空になります。
items === undefined の間は <></> を返して描画を止める設計のため、chrome.storage[area].get() が例外を投げた場合、void で飲み込まれて何のログも残らず、プロバイダ配下が一切描画されない状態が固定化されます。最低でも .catch() でエラー通知（console.error など）を行うことをおすすめします。onChanged リスナー側の再取得も同様です。
🛡️ 提案する修正
useEffect(() => {
let isMounted = true;
- void getItems().then((items) => {
- if (isMounted) setItems(items as T);
- });
+ getItems()
+ .then((items) => {
+ if (isMounted) setItems(items as T);
+ })
+ .catch((error: unknown) => {
+ console.error("Failed to load chrome.storage items", error);
+ });
return () => {
isMounted = false;
};
}, [getItems]);
useEffect(() => {
if (!watch) return;
const listener = () => {
- void getItems().then((items) => setItems(items as T));
+ getItems()
+ .then((items) => setItems(items as T))
+ .catch((error: unknown) => {
+ console.error("Failed to refresh chrome.storage items", error);
+ });
};
chrome.storage[area].onChanged.addListener(listener);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| let isMounted = true; | |
| void getItems().then((items) => { | |
| if (isMounted) setItems(items as T); | |
| }); | |
| return () => { | |
| isMounted = false; | |
| }; | |
| }, [getItems]); | |
| useEffect(() => { | |
| if (!watch) return; | |
| chrome.storage[area].onChanged.addListener(() => void load()); | |
| const listener = () => { | |
| void getItems().then((items) => setItems(items as T)); | |
| }; | |
| chrome.storage[area].onChanged.addListener(listener); | |
| return () => { | |
| chrome.storage[area].onChanged.removeListener(() => void load()); | |
| chrome.storage[area].onChanged.removeListener(listener); | |
| }; | |
| }, [load]); | |
| }, [getItems]); | |
| useEffect(() => { | |
| let isMounted = true; | |
| getItems() | |
| .then((items) => { | |
| if (isMounted) setItems(items as T); | |
| }) | |
| .catch((error: unknown) => { | |
| console.error("Failed to load chrome.storage items", error); | |
| }); | |
| return () => { | |
| isMounted = false; | |
| }; | |
| }, [getItems]); | |
| useEffect(() => { | |
| if (!watch) return; | |
| const listener = () => { | |
| getItems() | |
| .then((items) => setItems(items as T)) | |
| .catch((error: unknown) => { | |
| console.error("Failed to refresh chrome.storage items", error); | |
| }); | |
| }; | |
| chrome.storage[area].onChanged.addListener(listener); | |
| return () => { | |
| chrome.storage[area].onChanged.removeListener(listener); | |
| }; | |
| }, [getItems]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/contexts/ChromeStorageContext.tsx` around lines 45 - 64, The current
useEffect blocks call getItems() and swallow rejections (using void), which can
leave items undefined and the UI permanently blank; update both places that call
getItems() (the initial loader in the first useEffect and the onChanged listener
in the second useEffect) to attach a .catch handler that logs the error (e.g.,
console.error or processLogger.error) and optionally set a safe fallback via
setItems or leave previous state; ensure the listener still calls setItems on
success and that cleanup (removing the onChanged listener) remains unchanged.
Comment on lines
+150
to
+164
| <Button | ||
| isIconOnly | ||
| variant="tertiary" | ||
| size="sm" | ||
| className={ | ||
| isHovered || isSelected || starred || isLoading | ||
| ? "inline-flex" | ||
| : "hidden" | ||
| } | ||
| onPress={() => { | ||
| onStar(!starred); | ||
| }} | ||
| > | ||
| {starred ? <StarredIcon /> : <StarIcon />} | ||
| </Button> |
There was a problem hiding this comment.
Icon-only ボタンにアクセシブル名を追加してください。
isIconOnly のボタンがアイコンだけを描画しているため、支援技術では Star/Unstar の操作内容が伝わりません。aria-label を状態に応じて付けてください。
修正案
<Button
isIconOnly
+ aria-label={starred ? "Unstar" : "Star"}
variant="tertiary"
size="sm"
className={📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Button | |
| isIconOnly | |
| variant="tertiary" | |
| size="sm" | |
| className={ | |
| isHovered || isSelected || starred || isLoading | |
| ? "inline-flex" | |
| : "hidden" | |
| } | |
| onPress={() => { | |
| onStar(!starred); | |
| }} | |
| > | |
| {starred ? <StarredIcon /> : <StarIcon />} | |
| </Button> | |
| <Button | |
| isIconOnly | |
| aria-label={starred ? "Unstar" : "Star"} | |
| variant="tertiary" | |
| size="sm" | |
| className={ | |
| isHovered || isSelected || starred || isLoading | |
| ? "inline-flex" | |
| : "hidden" | |
| } | |
| onPress={() => { | |
| onStar(!starred); | |
| }} | |
| > | |
| {starred ? <StarredIcon /> : <StarIcon />} | |
| </Button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/GroupProjectList.tsx` around lines 150 - 164, The icon-only Button
rendering StarredIcon/StarIcon lacks an accessible name; update the Button (the
instance using props isIconOnly, variant="tertiary", size="sm", and onPress={()
=> onStar(!starred)}) to include an aria-label that reflects the current state
(e.g., "Unstar" when starred is true, "Star" when starred is false), so
assistive technologies can announce the action; ensure the label changes with
the starred boolean and is present alongside the existing props.
Comment on lines
57
to
70
| <Checkbox | ||
| id="auto-tab-switch" | ||
| isSelected={autoTabSwitch} | ||
| onValueChange={(isSelected) => void set({ autoTabSwitch: isSelected })} | ||
| onChange={(isSelected) => void set({ autoTabSwitch: isSelected })} | ||
| > | ||
| アイテム選択時に自動でタブを切り替える | ||
| <Checkbox.Control> | ||
| <Checkbox.Indicator /> | ||
| </Checkbox.Control> | ||
| <Checkbox.Content> | ||
| <Label htmlFor="auto-tab-switch"> | ||
| アイテム選択時に自動でタブを切り替える | ||
| </Label> | ||
| </Checkbox.Content> | ||
| </Checkbox> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
HeroUI v3 Checkbox Label htmlFor compound component association React Aria
💡 Result:
In HeroUI v3, the Checkbox is a compound component built on React Aria that uses a slotted anatomy rather than traditional htmlFor association for the Label. The recommended structure is: import { Checkbox, Label } from '@heroui/react'; <Checkbox.Control> <Checkbox.Indicator /> </Checkbox.Control> <Checkbox.Content> Accept terms and conditions </Checkbox.Content> Key points: - Provide an id prop to the Checkbox root (e.g., id="terms"). This id is automatically applied to the hidden native input element inside Checkbox.Control. - Place inside <Checkbox.Content>. The Label component supports htmlFor prop, but in this compound pattern, React Aria's context handles the association automatically without needing explicit htmlFor on the Label (Label uses for internally matching the input id). - The Label component docs confirm it renders a semantic HTML with htmlFor for association: Name. - This follows React Aria best practices for accessible compound components, ensuring clicking the label toggles the checkbox via ARIA linkage. - Inherits props from React Aria Checkbox, including aria-labelledby if needed for external labels, but slotted Label is preferred for compound use. Examples from docs show Terms (implicit slots) and explicit anatomy as above. No explicit htmlFor is shown on inner Label in anatomy, confirming automatic/contextual association via the shared id and React Aria's useLabel hook.
Citations:
- 1: https://heroui.com/docs/react/components/checkbox
- 2: https://v3.heroui.com/docs/react/components/checkbox
- 3: https://v3.heroui.com/docs/react/components/checkbox-group
- 4: https://heroui.com/docs/react/components/label
- 5: https://heroui-git-fix-sytles-exports-heroui.vercel.app/docs/react/components/checkbox
- 6: https://github.com/frontio-ai/heroui
Checkbox.Content 内の Label に htmlFor 属性を付与せず、Checkbox ルートの id も削除してください。
HeroUI v3 は React Aria Components ベースの複合構造を採用しており、Checkbox.Content 内に配置した Label はコンテキストを通じて自動的に関連付けられます。Checkbox ルートの id に加えて Label の htmlFor を両方指定することは冗長であり、React Aria のベストプラクティスでは <Label> は htmlFor なしで使用することが推奨されています。同じファイル内の Radio.Content と揃える形でリファクタリングしてください。
修正例
<Checkbox
- id="auto-tab-switch"
isSelected={autoTabSwitch}
onChange={(isSelected) => void set({ autoTabSwitch: isSelected })}
>
<Checkbox.Control>
<Checkbox.Indicator />
</Checkbox.Control>
<Checkbox.Content>
- <Label htmlFor="auto-tab-switch">
- アイテム選択時に自動でタブを切り替える
- </Label>
+ <Label>アイテム選択時に自動でタブを切り替える</Label>
</Checkbox.Content>
</Checkbox>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Checkbox | |
| id="auto-tab-switch" | |
| isSelected={autoTabSwitch} | |
| onValueChange={(isSelected) => void set({ autoTabSwitch: isSelected })} | |
| onChange={(isSelected) => void set({ autoTabSwitch: isSelected })} | |
| > | |
| アイテム選択時に自動でタブを切り替える | |
| <Checkbox.Control> | |
| <Checkbox.Indicator /> | |
| </Checkbox.Control> | |
| <Checkbox.Content> | |
| <Label htmlFor="auto-tab-switch"> | |
| アイテム選択時に自動でタブを切り替える | |
| </Label> | |
| </Checkbox.Content> | |
| </Checkbox> | |
| <Checkbox | |
| isSelected={autoTabSwitch} | |
| onChange={(isSelected) => void set({ autoTabSwitch: isSelected })} | |
| > | |
| <Checkbox.Control> | |
| <Checkbox.Indicator /> | |
| </Checkbox.Control> | |
| <Checkbox.Content> | |
| <Label>アイテム選択時に自動でタブを切り替える</Label> | |
| </Checkbox.Content> | |
| </Checkbox> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/options/App.tsx` around lines 57 - 70, Remove the redundant id on the
Checkbox root and the htmlFor on the Label inside Checkbox.Content: in the
Checkbox component where autoTabSwitch is used (the Checkbox with
id="auto-tab-switch"), delete the id prop and remove the
htmlFor="auto-tab-switch" from the Label inside Checkbox.Content so the Label
relies on HeroUI/React Aria context for association (match how Radio.Content is
implemented elsewhere).
hiroto7
force-pushed
the
codex-heroui-v3-migration
branch
from
April 18, 2026 12:48
d7bcf1a to
0481740
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/FeatureList.tsx (1)
284-288:⚠️ Potential issue | 🟡 Minor
Array.from()で互換性を改善してください。
keys.values().map(String).toArray()は Iterator Helpers (ES2026) を使用していますが、TypeScript target は ES2020 に設定されています。実際のランタイム（Chrome 122 以降）では互換性がありますが、設定の不整合があります。より安全で広く対応できるArray.from(keys, String)での実装をお勧めします。修正案
onExpandedChange={(keys: Set<Key>) => { if (!(keys instanceof Set)) return; void set({ - selectedFeatureListSections: keys.values().map(String).toArray(), + selectedFeatureListSections: Array.from(keys, String), }); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/FeatureList.tsx` around lines 284 - 288, Replace the use of Iterator Helpers with Array.from to avoid the ES2026 incompatibility: inside the onExpandedChange handler (the arrow function receiving keys), instead of using keys.values().map(String).toArray(), call Array.from(keys, String) and pass that result into the set call for selectedFeatureListSections; ensure you keep the runtime check (if (!(keys instanceof Set)) return) and update only the mapping expression used when invoking set.
♻️ Duplicate comments (2)
src/GroupProjectList.tsx (2)
150-164:⚠️ Potential issue | 🟠 MajorIcon-only ボタンにアクセシブル名を追加してください。
isIconOnlyのボタンがアイコンだけを描画しているため、支援技術では操作内容が伝わりません。状態に応じたaria-labelを追加してください。修正案
<Button isIconOnly + aria-label={starred ? "Unstar" : "Star"} variant="tertiary" size="sm" className={🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/GroupProjectList.tsx` around lines 150 - 164, The icon-only Button rendering inside GroupProjectList should include an accessible label so screen readers know its action and state; update the Button props (the isIconOnly Button that toggles starred state via onStar and displays StarIcon/StarredIcon) to add an aria-label that reflects whether the item is currently starred (e.g., "Unstar project" when starred is true, "Star project" when starred is false) and ensure the label updates when starred changes; keep the onPress behavior (onStar(!starred)) intact.
72-87:⚠️ Potential issue | 🟠 MajorLoading item の
id/keyをセクションごとに分けてください。Groups と Projects が同時に loading になると、同じ
ListBox内にid="skeleton"が2つ入ります。HeroUI の collection key 衝突を避けるため、section 別の loading key を使ってください。修正案
+ const groupLoadingKey = "skeleton-group"; + const projectLoadingKey = "skeleton-project"; + - const loadingItem = ( + const loadingItem = (id: string) => ( <ListBox.Item - key="skeleton" - id="skeleton" + key={id} + id={id} textValue="Loading..." className="flex items-center gap-2" > <Avatar size="sm" className="shrink-0 rounded-sm border"> <Avatar.Fallback> <Skeleton className="h-full w-full" /> </Avatar.Fallback> </Avatar> <Label className="w-full"> <Skeleton className="h-5 w-full" /> </Label> </ListBox.Item> );disabledKeys={ new Set([ - "skeleton", + groupLoadingKey, + projectLoadingKey, ...(isDraggingGroup<Header>Groups</Header> {groupItems.map((item) => { - if (item === "loading") return loadingItem; + if (item === "loading") return loadingItem(groupLoadingKey);<Header>Projects</Header> {projectItems.map((item) => { - if (item === "loading") return loadingItem; + if (item === "loading") return loadingItem(projectLoadingKey);Also applies to: 263-265, 286-324
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/GroupProjectList.tsx` around lines 72 - 87, The loading ListBox.Item uses non-unique key/id ("skeleton") which duplicates across sections (Groups and Projects) and causes collection key collisions; update the ListBox.Item instantiation(s) (the loadingItem constant and other similar ListBox.Item usages) to generate section-scoped unique keys/ids such as "skeleton-groups" and "skeleton-projects" (or append the section name/prop) so each section's loading item has a distinct key and id while keeping the same structure and textValue.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/FeatureList.tsx`:
- Around line 284-288: Replace the use of Iterator Helpers with Array.from to
avoid the ES2026 incompatibility: inside the onExpandedChange handler (the arrow
function receiving keys), instead of using keys.values().map(String).toArray(),
call Array.from(keys, String) and pass that result into the set call for
selectedFeatureListSections; ensure you keep the runtime check (if (!(keys
instanceof Set)) return) and update only the mapping expression used when
invoking set.
---
Duplicate comments:
In `@src/GroupProjectList.tsx`:
- Around line 150-164: The icon-only Button rendering inside GroupProjectList
should include an accessible label so screen readers know its action and state;
update the Button props (the isIconOnly Button that toggles starred state via
onStar and displays StarIcon/StarredIcon) to add an aria-label that reflects
whether the item is currently starred (e.g., "Unstar project" when starred is
true, "Star project" when starred is false) and ensure the label updates when
starred changes; keep the onPress behavior (onStar(!starred)) intact.
- Around line 72-87: The loading ListBox.Item uses non-unique key/id
("skeleton") which duplicates across sections (Groups and Projects) and causes
collection key collisions; update the ListBox.Item instantiation(s) (the
loadingItem constant and other similar ListBox.Item usages) to generate
section-scoped unique keys/ids such as "skeleton-groups" and "skeleton-projects"
(or append the section name/prop) so each section's loading item has a distinct
key and id while keeping the same structure and textValue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6d90ea4e-cea8-4155-8a96-b3db81cd47f5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
package.jsonsrc/App.tsxsrc/CustomAlert.tsxsrc/FeatureList.tsxsrc/GroupProjectList.tsxsrc/contexts/ChromeStorageContext.tsxsrc/hero.tssrc/index.csssrc/main.tsxsrc/options/App.tsxsrc/options/index.csssrc/options/main.tsx
💤 Files with no reviewable changes (1)
- src/hero.ts
✅ Files skipped from review due to trivial changes (3)
- src/main.tsx
- package.json
- src/App.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/index.css
- src/options/index.css
- src/options/main.tsx
- src/CustomAlert.tsx
- src/contexts/ChromeStorageContext.tsx
- src/options/App.tsx
hiroto7
force-pushed
the
codex-heroui-v3-migration
branch
6 times, most recently
from
April 18, 2026 13:36
69fedbd to
c5b6c9d
Compare
hiroto7
force-pushed
the
codex-heroui-v3-migration
branch
from
April 18, 2026 14:07
c5b6c9d to
1ed125c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@heroui/stylesimports.HeroUIProviderand the oldsrc/hero.tstheme plugin entry.ChromeStorageContextupdate.Why
HeroUI v3 is a larger migration than a dependency-only bump. This PR keeps the breaking API changes, dependency updates, and resulting UI adjustments together so the Dependabot bump can be reviewed or discarded separately.
Notes
@heroui/theme/ v2 Tailwind plugin usage is removed as part of the v3 styling migration.@heroui/reactand@heroui/stylesare updated to v3.framer-motionis removed because it is no longer required by this migration.main, including the pre-commit formatting hook merged in [codex] Add pre-commit formatting hook #216.Validation
pnpm exec prettier . --checkpnpm exec tsc -b --pretty falsepnpm exec eslint .pnpm test -- --run