[libs, ME]: persist records selection #669
Conversation
Angi-Kinas
commented
Nov 2, 2023
Angi-Kinas
commented
- Adds selection service (select records, deselect records, clear selection)
- Uses selection service in record-table
|
Affected libs:
|
| @@ -121,10 +135,15 @@ describe('RecordsListComponent', () => { | |||
| describe('when click on a record', () => { | |||
| beforeEach(() => { | |||
| table.recordSelect.emit({ uniqueIdentifier: 123 }) | |||
| table.recordsSelection.emit([{ uniqueIdentifier: 123 }]) | |||
There was a problem hiding this comment.
This deserves a separate test I think there should be a distinction between "clicking on a record" and "selecting a record"
| @Output() recordSelect = new EventEmitter<CatalogRecord>() | ||
| @Output() recordsSelection = new EventEmitter<CatalogRecord[]>() | ||
| @Output() recordsDeselection = new EventEmitter<CatalogRecord[]>() |
There was a problem hiding this comment.
I feel like the names of these events will bring confusion. How about:
recordClickrecordsSelectrecordsDeselect?
| handleRecordsSelection(records: CatalogRecord[]) { | ||
| this.selectionService | ||
| .selectRecords(records) | ||
| .pipe(takeUntil(this.onDestroy$)) |
There was a problem hiding this comment.
Interesting, that looks like a valid approach to avoid having long-running observables stopping after a component is destroyed. Here though we shouldn't need it as the selectRecords and deselectRecords of the service are built on top of HTTP calls, which means they are short-lived observables and will complete after their first emission.
| @@ -23,6 +24,7 @@ export class RecordTableComponent { | |||
| @Input() records: CatalogRecord[] | |||
| @Input() totalHits: number | |||
| @Output() recordSelect = new EventEmitter<CatalogRecord>() | |||
| @Output() recordsSelection = new EventEmitter<CatalogRecord[]>() | |||
There was a problem hiding this comment.
Is this output really needed? I didn't see any use of it
| @@ -23,6 +24,7 @@ export class RecordTableComponent { | |||
| @Input() records: CatalogRecord[] | |||
| @Input() totalHits: number | |||
| @Output() recordSelect = new EventEmitter<CatalogRecord>() | |||
There was a problem hiding this comment.
See my other comment about event names, this might deserve a clarification as well :)
