Add: Option to keep the selection menu open on tab/window change#431
Conversation
Adds startupMethod.keepMenuOpenOnTabChange, letting users keep the selection menu and selected text visible after running a command opens a new tab/window, so they can return and run another command on the same selection. Closes #365
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #431 +/- ##
==========================================
+ Coverage 29.93% 39.57% +9.63%
==========================================
Files 322 237 -85
Lines 33274 25349 -7925
Branches 1943 1886 -57
==========================================
+ Hits 9962 10031 +69
+ Misses 23312 15318 -7994 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
レビュー結果タブ/ウィンドウ切り替え時に選択メニューを維持するオプション追加、全体的に実装方針は明確で、既存のマイグレーションパターンや14言語ロケール更新もきちんと踏襲されています。以下、気になった点です。 1. (nit/確認) マイグレーションのバージョン閾値が未リリースのバージョンを指している
2. (テストカバレッジ) 新規ロジックに対するテストが不足
3. (パフォーマンス・軽微)
|
…page The Selection Command Hub app has migrated to a new repository (selection-command-hub) with a new architecture. packages/hub now only needs to host public/data/ai-services.json for the AiPrompt command and serve the e2e test page used by packages/extension's Playwright suite, so all other pages, components, features, and data have been removed. The extension's MyCommands widget (command_hub.tsx) only worked by injecting into hub pages that are now deleted, so it has been removed along with its manifest.json content_script entry and build config references. AGENTS.md and README.md have been updated to reflect the reduced scope. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fix the Header logo linking to a now-deleted route (404), trim locale dictionaries down to the keys actually consumed by the reduced hub app, drop dead code from useLocale and const.ts left over from the feature removal, and stop showing a Footer "Last Updated" date that was frozen to a stub value after GA integration was removed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # packages/extension/src/services/settings/settings.ts
Root causes were in BgData, not the onFocusChanged guard itself: - BgData.init() populated instance only via a fire-and-forget Storage.get().then(...), so any read/write before it resolved saw (or persisted) an incomplete state, silently dropping windowStack. - BgData.update()/set() read-then-wrote non-atomically, so a concurrent update (e.g. activeTabId tracking) could overwrite a just-added popup before it was ever read back. - The chrome.storage.onChanged echo listener could replay a stale write (including this context's own) after a newer local update, rolling back windowStack right after a popup was added. Fixes: synchronous default instance + ready() gate, atomic BgData.update()-based read-modify-write in WindowStackManager, and a revision counter so stale onChanged echoes are ignored. Also reorders onFocusChanged so popup-close scheduling isn't gated behind the settings fetch, and adds backgroundData.test.ts covering the fixed races. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
レビュー概要
良い点
気になる点
BgData.readyPromise = Storage.get<BgData>(SESSION_STORAGE_KEY.BG).then(
(val: BgData) => {
BgData.currentRevision = val?.revision ?? 0
BgData.instance = new BgData(val)
},
)
if (versionDiff(currentVersion, "1.1.1") === VersionDiff.Old) {
data = migrate1_1_1(data)
}
その他
セキュリティ面で懸念となる変更（外部入力の扱いなど）は見当たりませんでした。上記1・2は動作に影響し得るため、特にご確認いただけると幸いです。 |
The flag now gates menu-close behavior on both tab switches and side panel opens, not just tab changes, so rename it to reflect the broader focus-change scope. Updated all 14 locale messages to also mention the side panel.
レビュー結果PR #431（ 良い点
指摘事項1. 新機能の中核ロジックに対するユニットテストが見当たりません（テストカバレッジ）
2. 「メニューを強制的に閉じる」ロジックの重複（コード品質）
const settings = await enhancedSettings.getSection(CACHE_SECTIONS.USER_SETTINGS)
if (!settings.startupMethod?.keepMenuOpenOnFocusChange) {
try {
const ret = await Ipc.sendAllTab(TabCommand.closeMenu)
ret.filter((v) => v).forEach((v) => console.debug(v))
} catch (error) {
console.error("Failed to close menu:", error)
}
}同ファイル内で追加された 3. PR概要とコード上のオプション名の不一致（軽微） PR descriptionでは 4.
その他（軽微・任意）
セキュリティ今回の差分範囲では、外部入力の扱いやXSS/インジェクションに繋がるような変更は見当たりませんでした。 以上、よろしくお願いします。 |
migrate() runs while Storage.get is still in flight, so it must not trigger a write like Storage.setCommands mid-read. The migrated command added to data.commands is persisted by the caller once the read completes, so drop the incorrect expectation instead of adding back a write inside migrate1_1_0.
コードレビュー結果コマンド実行後にタブ/ウィンドウを切り替えても選択メニューを維持できるオプション追加、興味深い機能追加です。実装を確認しました。 レビュー範囲についての注意
気になった点
良い点
以上、よろしくお願いします。 |
1 participant
Summary
startupMethod.keepMenuOpenOnTabChangeオプション(デフォルトOFF)を有効にすると、選択メニューと選択テキストを維持するようにしました。chrome.tabs.onActivated/chrome.windows.onFocusChangedでのメニュー強制クローズ・選択テキストクリアを、オプション有効時はスキップします。Closes #365
Test plan
yarn test:run src/test/locales.test.ts— ロケールキーの整合性・順序を確認yarn tsc --noEmit— 型エラーなしyarn lint— 新規エラーなしyarn test:run(全体) — 882 passed。失敗14件は@supabase/supabase-js未インストール等の既存事象で、git stashにより変更前から存在することを確認済み