Feature: merge tool UI #789
Conversation
|
Oh, I guess I cant request a code review (I can on other repos) but consider this a request @jonaswinkler , whenever you have the chance obviously. And Im not sure why tests are failing... |
|
I'll get to it this weekend. |
Of course, take your time 🙂 |
| ngOnInit(): void { | ||
| this.list.selectNone() | ||
| this.list.activateSavedView(null) | ||
| this.list.reload() |
There was a problem hiding this comment.
Why do you need to do all this here? I've removed the init entirely and it feels quite natural.
| </div> | ||
| <div class="modal-body"> | ||
| <div class="m-n2 row row-cols-paperless-cards"> | ||
| <app-document-card-small [simpleCard]="true" [selected]="list.isSelected(d)" (toggleSelected)="toggleSelected(d, $event)" [document]="d" *ngFor="let d of list.documents"></app-document-card-small> |
There was a problem hiding this comment.
I'd probably do a separate selection model for the document chooser and not interfere with the list view at all.
This is also not very difficult, since there's no pagination / hidden items / filtering happenind in the chooser popup.
There was a problem hiding this comment.
Hmm, so to accomplish this would you basically copy the logic from the list view service e.g. toggleSelected, selectRangeTo and others directly into the chooser component? Just want to make sure I understand. That would mean the code essentially duplicated in two places but yes, would no longer mess with the main list.
There was a problem hiding this comment.
Oh right, range selection.
I'll take care of that.
| } | ||
|
|
||
| cancelClicked() { | ||
| this.list.selectNone() |
There was a problem hiding this comment.
Just as with the above, I'd not rely on the document list for selection state.
src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts
Outdated
Show resolved
Hide resolved
| modal.componentInstance.confirmClicked.subscribe(() => { | ||
| modal.componentInstance.buttonsEnabled = false | ||
| this.splitMergeService.addDocuments(this.list.selectedDocuments) | ||
| this.list.selectNone() |
There was a problem hiding this comment.
Again, I'd really prefer if this component did not mess with selection state of the list view.
|
Can I merge this? |
Sure, I guess youre saying you'd like to take care of the list abstraction thing (Im happy to do it if you want, just would need a little more direction), sounds good! |
This PR is
a work-in-progress foran initial build of a UI for the merge tool. See issue #335split_merge_ui_250321.mov
split_merge_3.mov