Skip to content

fix: ユーザースタイル削除時に別のスタイルが削除されるバグを修正#337

Merged
ujiro99 merged 5 commits intomainfrom
copilot/fix-user-style-deletion-issue
Mar 21, 2026
Merged

fix: ユーザースタイル削除時に別のスタイルが削除されるバグを修正#337
ujiro99 merged 5 commits intomainfrom
copilot/fix-user-style-deletion-issue

Conversation

Copy link
Contributor

Copilot AI commented Mar 19, 2026

フィルタリング後の配列インデックスを array.remove() に渡していたため、非表示スタイル(POPUP_DELAY, POPUP_DURATION)がある場合にインデックスがずれ、意図しないスタイルが削除されていた。

変更点

  • array.fields をフィルタリング後の index ではなく、field._id で元の配列から findIndex した originalIndexarray.remove() に渡すよう修正
  • findIndex-1 を返すケースのガードを追加
// Before: filteredIndex が元の配列のインデックスとずれる
.map((field, index) => (
  <RemoveButton onRemove={() => array.remove(index)} />
))

// After: _id で元の配列の正確なインデックスを解決
.map((field, filteredIndex) => {
  const originalIndex = array.fields.findIndex((f) => f._id === field._id)
  if (originalIndex === -1) return null
  return (
    <RemoveButton onRemove={() => array.remove(originalIndex)} />
  )
})
Original prompt

This section details on the original issue you should resolve

<issue_title>ユーザースタイルの削除時、別のスタイルが削除されてしまう</issue_title>
<issue_description>設定画面で、ユーザースタイルの削除を実行すると、指定した項目とは別の項目が削除されてしまう。</issue_description>

Comments on the Issue (you are @copilot in this section)


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Co-authored-by: ujiro99 <677231+ujiro99@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix user style deletion issue causing unintended removals fix: ユーザースタイル削除時に別のスタイルが削除されるバグを修正 Mar 19, 2026
Copilot AI requested a review from ujiro99 March 19, 2026 03:52
@ujiro99 ujiro99 marked this pull request as ready for review March 21, 2026 00:51
@codecov
Copy link

codecov bot commented Mar 21, 2026

Codecov Report

❌ Patch coverage is 0% with 35 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@97f0927). Learn more about missing BASE report.
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...ion/src/components/option/editor/UserStyleList.tsx 0.00% 35 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #337   +/-   ##
=======================================
  Coverage        ?   25.22%           
=======================================
  Files           ?      324           
  Lines           ?    31816           
  Branches        ?     1557           
=======================================
  Hits            ?     8027           
  Misses          ?    23789           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@claude
Copy link

claude bot commented Mar 21, 2026

PR レビュー

概要

このPRはユーザースタイル削除時に誤ったアイテムが削除されるバグ(#336)を修正しています。変更は1ファイル(packages/extension/src/components/option/editor/UserStyleList.tsx)に限定されており、ピンポイントな修正です。


良い点

1. バグ修正の根本的な解決

field._id を使って元の配列内の正しいインデックスを特定するアプローチは、問題の根本原因を正確に解消しています。

2. title属性のバグも同時修正(変更後 L183付近)

修正前: field.name をtitleに使用(内部フィールド名を表示)
修正後: fieldLabel をtitleに使用(翻訳済みラベルを表示)

これはUX上の改善にもなっており、副次的な修正として評価できます。

3. 翻訳関数の重複呼び出し排除

fieldLabel 変数に抽出したことで、同じ翻訳処理の二重呼び出しを防いでいます(変更後 L154-157付近)。


検討事項

1. originalIndex === -1 のガード節(変更後 L155)

対象: packages/extension/src/components/option/editor/UserStyleList.tsx L153-155
field は既に array.fields を元にフィルタリングされた要素なので、findIndex が -1 を返すことは論理的にありえません。ガード節は無害ですが、コメントで意図を明記することを推奨します。

2. テストの追加を推奨

UserStyleList コンポーネントのテストファイルが存在しません。
推奨ファイル: packages/extension/src/components/option/editor/UserStyleList.test.tsx
テスト内容: フィルタリングされる要素(POPUP_DELAY, POPUP_DURATION)が先頭にある場合でも、正しいインデックスのアイテムが削除されることを確認するテスト


パフォーマンス

map 内で毎回 findIndex(O(n))を呼び出すため O(n^2) ですが、小規模配列のため実用上問題ありません。


結論

バグ修正の内容は正確かつ適切です。ガード節へのコメント追加とテストの追加を検討していただければより完成度が上がります。全体的には問題なくマージ可能と考えます。

@ujiro99 ujiro99 merged commit 2cc7e2f into main Mar 21, 2026
6 checks passed
@ujiro99 ujiro99 deleted the copilot/fix-user-style-deletion-issue branch March 21, 2026 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ユーザースタイルの削除時、別のスタイルが削除されてしまう

2 participants