1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-07-07 12:25:46 +02:00

[PM-7683] Fix dynamic item defects (#9575)

* [PM-8639] Add data-testid attribute for test automation

* [PM-8669] Add autofill aria label

* [PM-8674] Show autofill menu options for card/identities when not in the autofill suggestion list

* [PM-8635] Hide menu items when copy cipher field directive is disabled

* [PM-8636] Disable copy menu dropdown when no items available to copy

* [CL-309] Add title override to bitBadge

* [PM-8669] Update menu-item directive disabled input

* [PM-7683] Fix race condition for remainingCiphers$

* [PM-7683] Use strict equality check
This commit is contained in:
Shane Melton 2024-06-12 14:33:18 -07:00 committed by GitHub
parent 7c16410c86
commit 6687ef5978
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 116 additions and 42 deletions

View File

@ -3323,16 +3323,6 @@
"clearFiltersOrTryAnother": {
"message": "Clear filters or try another search term"
},
"copyInfoLabel": {
"message": "Copy info, $ITEMNAME$",
"description": "Aria label for a button that opens a menu with options to copy information from an item.",
"placeholders": {
"itemname": {
"content": "$1",
"example": "Secret Item"
}
}
},
"copyInfoTitle": {
"message": "Copy info - $ITEMNAME$",
"description": "Title for a button that opens a menu with options to copy information from an item.",
@ -3343,16 +3333,6 @@
}
}
},
"copyNoteLabel": {
"message": "Copy Note, $ITEMNAME$",
"description": "Aria label for a button copies a note to the clipboard.",
"placeholders": {
"itemname": {
"content": "$1",
"example": "Secret Note Item"
}
}
},
"copyNoteTitle": {
"message": "Copy Note - $ITEMNAME$",
"description": "Title for a button copies a note to the clipboard.",
@ -3393,6 +3373,19 @@
}
}
},
"autofillTitle": {
"message": "Auto-fill - $ITEMNAME$",
"description": "Title for a button that auto-fills a login item.",
"placeholders": {
"itemname": {
"content": "$1",
"example": "Secret Item"
}
}
},
"noValuesToCopy": {
"message": "No values to copy"
},
"assignCollections": {
"message": "Assign collections"
},

View File

@ -3,8 +3,10 @@
type="button"
bitIconButton="bwi-clone"
size="small"
[attr.aria-label]="'copyInfoLabel' | i18n: cipher.name"
[title]="'copyInfoTitle' | i18n: cipher.name"
[appA11yTitle]="
hasLoginValues ? ('copyInfoTitle' | i18n: cipher.name) : ('noValuesToCopy' | i18n)
"
[disabled]="!hasLoginValues"
[bitMenuTriggerFor]="loginOptions"
></button>
<bit-menu #loginOptions>
@ -25,8 +27,10 @@
type="button"
bitIconButton="bwi-clone"
size="small"
[attr.aria-label]="'copyInfoLabel' | i18n: cipher.name"
[title]="'copyInfoTitle' | i18n: cipher.name"
[appA11yTitle]="
hasCardValues ? ('copyInfoTitle' | i18n: cipher.name) : ('noValuesToCopy' | i18n)
"
[disabled]="!hasCardValues"
[bitMenuTriggerFor]="cardOptions"
></button>
<bit-menu #cardOptions>
@ -44,8 +48,10 @@
type="button"
bitIconButton="bwi-clone"
size="small"
[attr.aria-label]="'copyInfoLabel' | i18n: cipher.name"
[title]="'copyInfoTitle' | i18n: cipher.name"
[appA11yTitle]="
hasIdentityValues ? ('copyInfoTitle' | i18n: cipher.name) : ('noValuesToCopy' | i18n)
"
[disabled]="!hasIdentityValues"
[bitMenuTriggerFor]="identityOptions"
></button>
<bit-menu #identityOptions>
@ -69,8 +75,9 @@
type="button"
bitIconButton="bwi-clone"
size="small"
[attr.aria-label]="'copyNoteLabel' | i18n: cipher.name"
[title]="'copyNoteTitle' | i18n: cipher.name"
[appA11yTitle]="
hasSecureNoteValue ? ('copyNoteTitle' | i18n: cipher.name) : ('noValuesToCopy' | i18n)
"
appCopyField="secureNote"
[cipher]="cipher"
></button>

View File

@ -25,5 +25,28 @@ export class ItemCopyActionsComponent {
protected CipherType = CipherType;
get hasLoginValues() {
return (
!!this.cipher.login.hasTotp || !!this.cipher.login.password || !!this.cipher.login.username
);
}
get hasCardValues() {
return !!this.cipher.card.code || !!this.cipher.card.number;
}
get hasIdentityValues() {
return (
!!this.cipher.identity.fullAddressForCopy ||
!!this.cipher.identity.email ||
!!this.cipher.identity.username ||
!!this.cipher.identity.phone
);
}
get hasSecureNoteValue() {
return !!this.cipher.notes;
}
constructor() {}
}

View File

@ -8,7 +8,7 @@
[bitMenuTriggerFor]="moreOptions"
></button>
<bit-menu #moreOptions>
<ng-container *ngIf="isLogin && !hideLoginOptions">
<ng-container *ngIf="canAutofill && !hideAutofillOptions">
<ng-container *ngIf="autofillAllowed$ | async">
<button type="button" bitMenuItem>
{{ "autofill" | i18n }}

View File

@ -33,11 +33,11 @@ export class ItemMoreOptionsComponent {
cipher: CipherView;
/**
* Flag to hide the login specific menu options. Used for login items that are
* Flag to hide the autofill menu options. Used for items that are
* already in the autofill list suggestion.
*/
@Input({ transform: booleanAttribute })
hideLoginOptions: boolean;
hideAutofillOptions: boolean;
protected autofillAllowed$ = this.vaultPopupItemsService.autofillAllowed$;
@ -55,8 +55,11 @@ export class ItemMoreOptionsComponent {
return this.cipher.edit;
}
get isLogin() {
return this.cipher.type === CipherType.Login;
/**
* Determines if the cipher can be autofilled.
*/
get canAutofill() {
return [CipherType.Login, CipherType.Card, CipherType.Identity].includes(this.cipher.type);
}
get favoriteText() {
@ -67,7 +70,7 @@ export class ItemMoreOptionsComponent {
* Determines if the login cipher can be launched in a new browser tab.
*/
get canLaunch() {
return this.isLogin && this.cipher.login.canLaunch;
return this.cipher.type === CipherType.Login && this.cipher.login.canLaunch;
}
/**

View File

@ -25,7 +25,7 @@
[appA11yTitle]="'viewItemTitle' | i18n: cipher.name"
>
<app-vault-icon slot="start" [cipher]="cipher"></app-vault-icon>
{{ cipher.name }}
<span data-testid="item-name">{{ cipher.name }}</span>
<i
class="bwi bwi-sm"
*ngIf="cipher.organizationId"
@ -36,12 +36,20 @@
</a>
<ng-container slot="end">
<bit-item-action *ngIf="showAutofillButton">
<button type="button" bitBadge variant="primary">{{ "autoFill" | i18n }}</button>
<button
type="button"
bitBadge
variant="primary"
[title]="'autofillTitle' | i18n: cipher.name"
[attr.aria-label]="'autofillTitle' | i18n: cipher.name"
>
{{ "autoFill" | i18n }}
</button>
</bit-item-action>
<app-item-copy-actions [cipher]="cipher"></app-item-copy-actions>
<app-item-more-options
[cipher]="cipher"
[hideLoginOptions]="showAutofillButton"
[hideAutofillOptions]="showAutofillButton"
></app-item-more-options>
</ng-container>
</bit-item>

View File

@ -2,6 +2,7 @@ import { inject, Injectable, NgZone } from "@angular/core";
import {
BehaviorSubject,
combineLatest,
concatMap,
distinctUntilChanged,
distinctUntilKeyChanged,
from,
@ -176,7 +177,12 @@ export class VaultPopupItemsService {
* Ciphers are sorted by name.
*/
remainingCiphers$: Observable<PopupCipherView[]> = this.favoriteCiphers$.pipe(
withLatestFrom(this._filteredCipherList$, this.autoFillCiphers$),
concatMap(
(
favoriteCiphers, // concatMap->of is used to make withLatestFrom lazy to avoid race conditions with autoFillCiphers$
) =>
of(favoriteCiphers).pipe(withLatestFrom(this._filteredCipherList$, this.autoFillCiphers$)),
),
map(([favoriteCiphers, ciphers, autoFillCiphers]) =>
ciphers.filter(
(cipher) => !autoFillCiphers.includes(cipher) && !favoriteCiphers.includes(cipher),

View File

@ -51,10 +51,18 @@ export class BadgeDirective implements FocusableElement {
.concat(this.hasHoverEffects ? hoverStyles[this.variant] : [])
.concat(this.truncate ? ["tw-truncate", this.maxWidthClass] : []);
}
@HostBinding("attr.title") get title() {
@HostBinding("attr.title") get titleAttr() {
if (this.title !== undefined) {
return this.title;
}
return this.truncate ? this.el.nativeElement.textContent.trim() : null;
}
/**
* Optional override for the automatic badge title when truncating.
*/
@Input() title?: string;
/**
* Variant, sets the background color of the badge.
*/

View File

@ -1,5 +1,6 @@
import { FocusableOption } from "@angular/cdk/a11y";
import { Component, ElementRef, HostBinding } from "@angular/core";
import { coerceBooleanProperty } from "@angular/cdk/coercion";
import { Component, ElementRef, HostBinding, Input } from "@angular/core";
@Component({
selector: "[bitMenuItem]",
@ -32,6 +33,11 @@ export class MenuItemDirective implements FocusableOption {
];
@HostBinding("attr.role") role = "menuitem";
@HostBinding("tabIndex") tabIndex = "-1";
@HostBinding("attr.disabled") get disabledAttr() {
return this.disabled || null; // native disabled attr must be null when false
}
@Input({ transform: coerceBooleanProperty }) disabled?: boolean = false;
constructor(private elementRef: ElementRef) {}

View File

@ -1,6 +1,7 @@
import { Directive, HostBinding, HostListener, Input, OnChanges } from "@angular/core";
import { Directive, HostBinding, HostListener, Input, OnChanges, Optional } from "@angular/core";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { MenuItemDirective } from "@bitwarden/components";
import { CopyAction, CopyCipherFieldService } from "@bitwarden/vault";
/**
@ -9,6 +10,8 @@ import { CopyAction, CopyCipherFieldService } from "@bitwarden/vault";
*
* Automatically disables the host element if the field to copy is not available or null.
*
* If the host element is a menu item, it will be hidden when disabled.
*
* @example
* ```html
* <button appCopyField="username" [cipher]="cipher">Copy Username</button>
@ -27,11 +30,23 @@ export class CopyCipherFieldDirective implements OnChanges {
@Input({ required: true }) cipher: CipherView;
constructor(private copyCipherFieldService: CopyCipherFieldService) {}
constructor(
private copyCipherFieldService: CopyCipherFieldService,
@Optional() private menuItemDirective?: MenuItemDirective,
) {}
@HostBinding("attr.disabled")
protected disabled: boolean | null = null;
/**
* Hide the element if it is disabled and is a menu item.
* @private
*/
@HostBinding("class.tw-hidden")
private get hidden() {
return this.disabled && this.menuItemDirective;
}
@HostListener("click")
async copy() {
const value = this.getValueToCopy();
@ -49,6 +64,11 @@ export class CopyCipherFieldDirective implements OnChanges {
(this.action === "totp" && !(await this.copyCipherFieldService.totpAllowed(this.cipher)))
? true
: null;
// If the directive is used on a menu item, update the menu item to prevent keyboard navigation
if (this.menuItemDirective) {
this.menuItemDirective.disabled = this.disabled;
}
}
private getValueToCopy() {