1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-10 21:50:15 +00:00

[PM-26195] Refactor selection logic in VaultItemsComponent to ensure bulk select visual bug fix (#18674)

* Refactor selection logic in VaultItemsComponent to ensure bulk selection matches visual order. Added getSortedEditableItems method for sorting based on current table configuration, enhancing user experience during selection.

* Refactor sorting logic in VaultItemsComponent to clarify collection prioritization. Updated comments to indicate that collection comparison is direction-independent, enhancing code readability and maintainability.
This commit is contained in:
Jared
2026-02-09 13:46:17 -05:00
committed by GitHub
parent 47d1025d27
commit da219b911e

View File

@@ -260,9 +260,9 @@ export class VaultItemsComponent<C extends CipherViewLike> {
}
get isAllSelected() {
return this.editableItems
.slice(0, MaxSelectionCount)
.every((item) => this.selection.isSelected(item));
// Check selection against sorted items to match toggleAll() behavior
const sortedItems = this.getSortedEditableItems();
return sortedItems.slice(0, MaxSelectionCount).every((item) => this.selection.isSelected(item));
}
get isEmpty() {
@@ -376,9 +376,30 @@ export class VaultItemsComponent<C extends CipherViewLike> {
}
protected toggleAll() {
this.isAllSelected
? this.selection.clear()
: this.selection.select(...this.editableItems.slice(0, MaxSelectionCount));
if (this.isAllSelected) {
this.selection.clear();
} else {
const sortedItems = this.getSortedEditableItems();
this.selection.select(...sortedItems.slice(0, MaxSelectionCount));
}
}
/**
* Returns editableItems sorted according to the current table sort configuration.
* This ensures bulk selection matches the visual order displayed to the user.
*/
private getSortedEditableItems(): VaultItem<C>[] {
const currentSort = this.dataSource.sort;
const items = [...this.editableItems];
// If no sort function is set, return items in their original order (as displayed in table)
if (!currentSort || !currentSort.fn) {
return items;
}
// Apply sort function with direction modifier (matches TableDataSource.sortData behavior)
const directionModifier = currentSort.direction === "asc" ? 1 : -1;
return items.sort((a, b) => currentSort.fn(a, b, currentSort.direction) * directionModifier);
}
protected event(event: VaultItemEvent<C>) {
@@ -584,12 +605,13 @@ export class VaultItemsComponent<C extends CipherViewLike> {
* Sorts VaultItems, grouping collections before ciphers, and sorting each group alphabetically by name.
*/
protected sortByName = (a: VaultItem<C>, b: VaultItem<C>, direction: SortDirection) => {
// Collections before ciphers
const collectionCompare = this.prioritizeCollections(a, b, direction);
// Collections before ciphers (direction-independent)
const collectionCompare = this.prioritizeCollections(a, b);
if (collectionCompare !== 0) {
return collectionCompare;
}
// Name comparison (direction-dependent, handled by directionModifier)
return this.compareNames(a, b);
};
@@ -611,8 +633,8 @@ export class VaultItemsComponent<C extends CipherViewLike> {
return null;
};
// Collections before ciphers
const collectionCompare = this.prioritizeCollections(a, b, direction);
// Collections before ciphers (direction-independent)
const collectionCompare = this.prioritizeCollections(a, b);
if (collectionCompare !== 0) {
return collectionCompare;
}
@@ -655,8 +677,8 @@ export class VaultItemsComponent<C extends CipherViewLike> {
return priorityMap[permission] ?? -1;
};
// Collections before ciphers
const collectionCompare = this.prioritizeCollections(a, b, direction);
// Collections before ciphers (direction-independent)
const collectionCompare = this.prioritizeCollections(a, b);
if (collectionCompare !== 0) {
return collectionCompare;
}
@@ -664,11 +686,12 @@ export class VaultItemsComponent<C extends CipherViewLike> {
const priorityA = getPermissionPriority(a);
const priorityB = getPermissionPriority(b);
// Higher priority first
// Higher priority first (direction-dependent, handled by directionModifier)
if (priorityA !== priorityB) {
return priorityA - priorityB;
}
// Fallback to name comparison (direction-dependent, handled by directionModifier)
return this.compareNames(a, b);
};
@@ -679,22 +702,19 @@ export class VaultItemsComponent<C extends CipherViewLike> {
/**
* Sorts VaultItems by prioritizing collections over ciphers.
* Collections are always placed before ciphers, regardless of the sorting direction.
* Always returns -1 for collections before ciphers, regardless of sort direction.
* This comparison is direction-independent; the direction is applied separately via directionModifier.
*/
private prioritizeCollections(
a: VaultItem<C>,
b: VaultItem<C>,
direction: SortDirection,
): number {
private prioritizeCollections(a: VaultItem<C>, b: VaultItem<C>): number {
if (a.collection && !b.collection) {
return direction === "asc" ? -1 : 1;
return -1; // a (collection) comes before b (cipher)
}
if (!a.collection && b.collection) {
return direction === "asc" ? 1 : -1;
return 1; // b (collection) comes before a (cipher)
}
return 0;
return 0; // Both are collections or both are ciphers
}
private hasPersonalItems(): boolean {