-
Notifications
You must be signed in to change notification settings - Fork 24
Add split button to 'create dialog' #663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: future
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, just some small remarks to clean up 😊 Feel free to rebase and squash your changes afterwards, then we will merge them 👍
matInput | ||
[formControl]="operationNumberFormControl" | ||
[(ngModel)]="operationNumber" | ||
[disabled]="isLoading()" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not needed anymore? if yes, why?
<mat-menu #actionMenu="matMenu"> | ||
<button | ||
mat-menu-item | ||
(click)="onSelectAction('ADD_CREATE')" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would make a nice little enum, don't you think? 🤔
if (!failed) this.dialog.close(); | ||
} | ||
|
||
primaryActionLabel = computed(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fields belong at the beginning of the class to the others 😊
} | ||
|
||
this.operationNumberFormControl.reset(''); | ||
this.operationNumberFormControl.markAsPristine(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this and the following line should be redundant, see https://angular.dev/api/forms/FormControl -> reset
Also check line 208, that shouldn't be necessary anymore either
if (this.operations().length > 0) await this.create(); | ||
return; | ||
} | ||
if (this.canAddOperation()) this.addOperation(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
either make this if-else and remove the return or use the early return and remove the second if, this is half of both options 😅
Summary
This PR replaces the old
+
button with a split button that exposes two actions:Add + Create
under the hood. It adds the current operation (if valid) and immediately creates all queued operations. If the form is invalid but there are already queued operations, it still creates those.The right-hand arrow only selects which action the left button executes. Selection doesn't execute. This streamlines the common case (one order + one operation) while keeping the rare multi-operation flow via
Add
.Screenshots