[PM-12505] - add delete send button to footer #11187
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #11187 +/- ##
==========================================
+ Coverage 34.98% 35.08% +0.09%
==========================================
Files 2712 2718 +6
Lines 84577 84738 +161
Branches 16069 16110 +41
==========================================
+ Hits 29587 29728 +141
- Misses 54019 54038 +19
- Partials 971 972 +1 ☔ View full report in Codecov by Sentry. |
|
No New Or Fixed Issues Found |
There was a problem hiding this comment.
As a quick win (approaching deadline), we can include the delete within the send-add-edit.component.
Thinking forward though about re-using the send-form.component on other clients, it should take care of the deletion (via SendFormConfigService) and prompt like it already does for the submit action.
I envision a onSendDeleted-event which would then take care of the navigation and success toast, which are client-specific. On browser I'd navigate back (this.location.back) but on web I'd close a dialog and on desktop I might remove/hide the tools-send-form 🤷
| async deleteSend() { | ||
| const confirmed = await this.dialogService.openSimpleDialog({ | ||
| title: { key: "deleteSend" }, | ||
| content: { key: "deleteSendConfirmation" }, |
There was a problem hiding this comment.
The message needs to be changed according to the design into:
"Are you sure you want to permanently delete this Send?"
While this is being changed, it should also be used within the send-list-items-container.component
| *ngIf="config?.mode !== 'add'" | ||
| type="button" | ||
| buttonType="danger" | ||
| class="tw-border-0 tw-ml-auto bwi-lg" |
There was a problem hiding this comment.
The classes shouldn't be need
There was a problem hiding this comment.
@djsmith85 Without the classes the icon gets a border, doesn't align right, and is too small. I'm open to suggestion on how you'd prefer to implement these requirements.
There was a problem hiding this comment.
@danielleflinn As mentioned on another comment I was looking at vault's view2 component for how the delete button was implemented there
There was a problem hiding this comment.
This component should use https://components.bitwarden.com/?path=/story/component-library-icon-button--danger, in the current styles it has a border which will be removed in the Design System feature branch.
There was a problem hiding this comment.
Ok I'll remove the tw-border-0 class. According to @danielleflinn's feedback everything else should remain unless I'm missing something.
| buttonType="danger" | ||
| class="tw-border-0 tw-ml-auto bwi-lg" | ||
| bitIconButton="bwi-trash" | ||
| (click)="deleteSend()" |
There was a problem hiding this comment.
Please use bitAction instead. Example for the delete button can be found on the vault's view2 component
There was a problem hiding this comment.
Approving for the changes I requested, leaving it to @danielleflinn to comment/approve on the usage of the classes.
| @@ -92,6 +104,37 @@ export class SendAddEditComponent { | |||
| this.location.back(); | |||
| } | |||
|
|
|||
| async deleteSend() { | |||
There was a problem hiding this comment.
delete = async (): Promise<boolean> => {
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-12505
📔 Objective
This PR adds the delete button to the footer of the edit send page.
📸 Screenshots
Screen.Recording.2024-09-20.at.4.38.21.PM.mov
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes