fix: removed delete confirmation check dialogue#8417
Conversation
jaycnz
added
type: fix
WalkthroughThe DeleteBlock confirmation dialog and its state/handlers were removed; delete actions now execute immediately. Tests were updated to stop interacting with a confirmation dialog and now assert the deletion mutation and cache changes directly. Locale entries for card deletion strings were removed or relocated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DeleteBlock as DeleteBlock Component
participant API as Delete Mutation
participant Cache
rect rgb(200,220,255)
note over User,DeleteBlock: Before (old flow with confirmation)
User->>DeleteBlock: Click delete
DeleteBlock->>DeleteBlock: Open confirmation dialog
User->>DeleteBlock: Confirm deletion
DeleteBlock->>API: Call delete mutation
API-->>Cache: Update cache
end
rect rgb(220,255,220)
note over User,DeleteBlock: After (new immediate flow)
User->>DeleteBlock: Click delete
DeleteBlock->>API: Call delete mutation immediately
API-->>Cache: Update cache
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-09-12T20:30:23.683ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
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 |
|
View your CI Pipeline Execution ↗ for commit 4e5412d
☁️ Nx Cloud last updated this comment at |
github-actions
bot
requested a deployment
to
Preview - journeys-admin
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/journeys-admin/src/components/Editor/Slider/Content/Canvas/QuickControls/DeleteBlock/DeleteBlock.tsx (1)
42-70: Consider removing the unnecessary fragment wrapper.The fragment wrapper is no longer needed since it only contains a single child (the conditional render). Removing it would simplify the code.
Apply this diff to remove the fragment:
return ( - <> - {variant === 'button' ? ( - <IconButton - id="delete-block-actions" - aria-label="Delete Block Actions" - aria-controls="delete-block-actions" - aria-haspopup="true" - aria-expanded="true" - disabled={disableAction} - onMouseUp={handleDeleteBlock} - sx={{ - p: 1 - }} - > - <Trash2Icon /> - </IconButton> - ) : ( - <MenuItem - label={t('Delete {{ label }}', { - label: blockType === 'Card' ? t('Card') : t('Block') - })} - icon={<Trash2Icon />} - disabled={disableAction} - onClick={handleDeleteBlock} - /> - )} - </> + variant === 'button' ? ( + <IconButton + id="delete-block-actions" + aria-label="Delete Block Actions" + aria-controls="delete-block-actions" + aria-haspopup="true" + aria-expanded="true" + disabled={disableAction} + onMouseUp={handleDeleteBlock} + sx={{ + p: 1 + }} + > + <Trash2Icon /> + </IconButton> + ) : ( + <MenuItem + label={t('Delete {{ label }}', { + label: blockType === 'Card' ? t('Card') : t('Block') + })} + icon={<Trash2Icon />} + disabled={disableAction} + onClick={handleDeleteBlock} + /> + ) ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/journeys-admin/src/components/Editor/Slider/Content/Canvas/QuickControls/DeleteBlock/DeleteBlock.spec.tsx(0 hunks)apps/journeys-admin/src/components/Editor/Slider/Content/Canvas/QuickControls/DeleteBlock/DeleteBlock.tsx(4 hunks)
💤 Files with no reviewable changes (1)
- apps/journeys-admin/src/components/Editor/Slider/Content/Canvas/QuickControls/DeleteBlock/DeleteBlock.spec.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apps/journeys-admin/src/components/Editor/Slider/Content/Canvas/QuickControls/DeleteBlock/DeleteBlock.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apps/journeys-admin/src/components/Editor/Slider/Content/Canvas/QuickControls/DeleteBlock/DeleteBlock.tsx
apps/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/apps.mdc)
apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Files:
apps/journeys-admin/src/components/Editor/Slider/Content/Canvas/QuickControls/DeleteBlock/DeleteBlock.tsx
🧠 Learnings (3)
📚 Learning: 2025-09-10T19:52:52.934Z
Learnt from: mikeallisonJS
Repo: JesusFilm/core PR: 7629
File: apis/api-journeys/schema.graphql:947-956
Timestamp: 2025-09-10T19:52:52.934Z
Learning: The canBlockHaveAction function in apis/api-journeys/src/app/modules/action/canBlockHaveAction/canBlockHaveAction.ts incorrectly includes 'MultiselectOptionBlock' in its allowlist, but MultiselectOptionBlock blocks should not have actions. Only MultiselectBlock (the container) should have actions, not the individual option blocks.
Applied to files:
apps/journeys-admin/src/components/Editor/Slider/Content/Canvas/QuickControls/DeleteBlock/DeleteBlock.tsx
📚 Learning: 2025-09-19T18:48:41.906Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-09-19T18:48:41.906Z
Learning: Applies to apps/watch/**/*.{tsx} : Dialogs/Modals prefixed with 'Dialog', Sections with 'Section', Layout components with 'Layout'
Applied to files:
apps/journeys-admin/src/components/Editor/Slider/Content/Canvas/QuickControls/DeleteBlock/DeleteBlock.tsx
📚 Learning: 2025-09-19T18:48:41.906Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-09-19T18:48:41.906Z
Learning: Applies to apps/watch/src/**/*.{tsx} : Navigation: design link-based content discovery through components
Applied to files:
apps/journeys-admin/src/components/Editor/Slider/Content/Canvas/QuickControls/DeleteBlock/DeleteBlock.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Deploy Preview (journeys-admin, 8417/merge, pull_request, 22)
- GitHub Check: test (22, 3/3)
- GitHub Check: test (22, 2/3)
- GitHub Check: build (22)
- GitHub Check: lint (22)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/journeys-admin/src/components/Editor/Slider/Content/Canvas/QuickControls/DeleteBlock/DeleteBlock.tsx (2)
3-3: LGTM: Import cleanup is correct.The import statement now includes only
ReactElement, which is appropriate after removing the confirmation dialog.
37-40: Based on my verification, I have found that the codebase has a comprehensive undo/redo system that fully addresses the concerns raised in the review comment. TheDeleteBlockcomponent integrates withuseBlockDeleteCommand, which uses the command pattern to register delete operations with full undo capability.Key findings:
- Undo mechanism exists: The
useBlockDeleteCommandhook registers deletion commands withuseCommand(), including bothexecute()andundo()implementations- Block restore capability: A
blockRestoremutation handles undoing deletions with proper state restoration- Multiple undo methods available: Users can undo via:
- Toolbar Undo button (
CommandUndoItem)- Keyboard shortcut (Mod+Z)
- Redo button (
CommandRedoItem)- Tests confirm functionality: DeleteBlock.spec.tsx includes test "should restore card on undo click" (line 380)
The immediate deletion without confirmation is safe and intentional, aligned with the PR title "fix: removed delete confirmation check dialogue". Users have full recovery capability through the built-in undo system.
Immediate deletion is intentional and safe due to built-in undo/redo mechanism.
The
addBlockDeletefunction integrates with the command pattern (useBlockDeleteCommandhook) which automatically registers deletion operations with full undo capability. Users can recover deleted blocks via:
- Toolbar Undo button
- Keyboard shortcut (Ctrl+Z / Cmd+Z)
- Redo button in toolbar
The command pattern ensures proper state management and restoration, making accidental data loss recoverable.
...-admin/src/components/Editor/Slider/Content/Canvas/QuickControls/DeleteBlock/DeleteBlock.tsx
Show resolved
Hide resolved
github-actions
bot
temporarily deployed
to
Preview - journeys-admin
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
jaycnz
deleted the
jaychen/nes-1013-existing-feature-improvement-delete-confirmation-check
branch
2 participants
Summary by CodeRabbit
Updated Features
Documentation (Localization)
Tests
✏️ Tip: You can customize this high-level summary in your review settings.