fix(desktop): カスタム通知音の削除確認をカスタムダイアログ化#265
Conversation
OS デフォルトの window.confirm ではなく、リネームと同じ Dialog コンポーネントで 削除確認 UI を提供する。 Closes #262
|
@codex review |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 24 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughリングトーン削除時の確認フローをネイティブダイアログからカスタムUIコンポーネント Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ 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". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/components/DeleteRingtoneDialog/DeleteRingtoneDialog.tsx (1)
29-31:handleConfirmラッパーの必要性現状
handleConfirmはonConfirm()を単にawaitしているだけで、エラーハンドリングや追加処理を行っていません。onClick={handleConfirm}をonClick={() => { void onConfirm(); }}または直接onConfirmを渡す形でも等価です。ラッパーを残すなら将来の拡張ポイントであることをコメントで示すとより意図が明確になります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/components/DeleteRingtoneDialog/DeleteRingtoneDialog.tsx` around lines 29 - 31, The handleConfirm wrapper currently only awaits onConfirm() without adding behavior; either remove handleConfirm and pass onConfirm directly to the onClick handler (or use onClick={() => { void onConfirm(); }}) or keep handleConfirm but add a brief comment above it explaining it's an explicit extension point for future error handling/extra logic; update the component to use the chosen approach and ensure references to handleConfirm and onConfirm in the JSX are adjusted accordingly.apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/RingtonesSettings.tsx (1)
500-510: 送信中のダイアログ外クローズ抑止の検討
isSubmitting中でも Esc キーやオーバーレイクリックでonOpenChange(false)が発火し、削除処理進行中にダイアログが閉じてdeleteErrorもクリアされてしまいます。Cancel ボタンはdisabledにしているので、挙動の一貫性のためにも送信中の閉じ操作は抑止するのが安全です。RenameRingtoneDialog/YouTubeImportDialogも同様のパターンなので、合わせて検討してもよいかもしれません。💡 提案 (例)
<DeleteRingtoneDialog open={deleteDialogOpen} onOpenChange={(open) => { + if (!open && deleteCustomRingtone.isPending) return; setDeleteDialogOpen(open); if (!open) setDeleteError(null); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/RingtonesSettings.tsx` around lines 500 - 510, The DeleteRingtoneDialog can be closed via overlay click or Esc while a delete is in progress which also clears deleteError; update the onOpenChange handling so attempts to close (open === false) are ignored when deleteCustomRingtone.isPending is true (i.e., keep deleteDialogOpen true and do not call setDeleteError(null) while submitting), or alternatively add and pass dialog props (e.g., disableBackdropClick/disableEscapeKeyDown or a modal blocking prop) from RingtonesSettings into DeleteRingtoneDialog so it cannot be dismissed during isSubmitting; apply the same pattern to RenameRingtoneDialog and YouTubeImportDialog for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/components/DeleteRingtoneDialog/DeleteRingtoneDialog.tsx`:
- Around line 29-31: The handleConfirm wrapper currently only awaits onConfirm()
without adding behavior; either remove handleConfirm and pass onConfirm directly
to the onClick handler (or use onClick={() => { void onConfirm(); }}) or keep
handleConfirm but add a brief comment above it explaining it's an explicit
extension point for future error handling/extra logic; update the component to
use the chosen approach and ensure references to handleConfirm and onConfirm in
the JSX are adjusted accordingly.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/RingtonesSettings.tsx`:
- Around line 500-510: The DeleteRingtoneDialog can be closed via overlay click
or Esc while a delete is in progress which also clears deleteError; update the
onOpenChange handling so attempts to close (open === false) are ignored when
deleteCustomRingtone.isPending is true (i.e., keep deleteDialogOpen true and do
not call setDeleteError(null) while submitting), or alternatively add and pass
dialog props (e.g., disableBackdropClick/disableEscapeKeyDown or a modal
blocking prop) from RingtonesSettings into DeleteRingtoneDialog so it cannot be
dismissed during isSubmitting; apply the same pattern to RenameRingtoneDialog
and YouTubeImportDialog for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 75f01e98-0a54-4b09-87a9-28a7eda3afbf
📒 Files selected for processing (3)
apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/RingtonesSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/components/DeleteRingtoneDialog/DeleteRingtoneDialog.tsxapps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/components/DeleteRingtoneDialog/index.ts
CodeRabbit レビュー対応: - 送信中の Esc / オーバーレイクリックでダイアログが閉じないようガード - 不要な handleConfirm ラッパーを除去し onConfirm を直接呼び出す
1 participant
概要
Closes #262
カスタム通知音の削除時に表示されるダイアログが、OS / ブラウザのデフォルト確認ダイアログ (window.confirm) になっていたため、リネーム時と同じ shadcn の Dialog コンポーネントで統一する。
変更点
DeleteRingtoneDialogを新規追加（Cancel / Delete ボタン構成、destructive バリアント）RingtonesSettingsのhandleDeleteCustomを window.confirm から新ダイアログ表示に置換テスト計画
Summary by CodeRabbit
リリースノート