[PM-4871] Remove clickable row in favor of clickable cell content #6911

Spitfireap · 2023-11-19T21:31:08Z

Type of change

- [ ] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ X] UX Improvement

Objective

Improve UX.

When selecting multiple items in the vault it is easy to mis-click and trigger the "show item" action which not only show the item information but also uncheck every previously checked checkbox. It is really hard to manage a great number of items.

The proposed change check the click event and if the mouse is above the checkbox's <td> the checkbox will trigger instead.

Forum link referring to this issue.

Code changes

vault-cipher-row.component.html: added an id to the <td> containing the checkbox for the click event check
vault-cipher-row.component.ts: Updated the click event by providing the event's target. If the target contains the id, the input is then checked.

Screenshots

old behavior:

old.mp4

new behavior:

new.mp4

bitwarden-bot · 2023-11-19T21:31:53Z

Thank you for your contribution! We've added this to our internal Community PR board for review.
ID: PM-4871

bitwarden-bot · 2023-11-19T22:54:29Z

Checkmarx One – Scan Summary & Details – c494d3cd-2c9e-4d4e-8549-6b1aca35c0c7

New Issues

Issue	Source File / Package	Checkmarx Insight
Unpinned Actions Full Length Commit SHA	/build-desktop.yml: 1163	Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
Unpinned Actions Full Length Commit SHA	/build-desktop.yml: 299	Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
Unpinned Actions Full Length Commit SHA	/build-desktop.yml: 1242	Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
Use_Of_Hardcoded_Password	/apps/cli/src/tools/generate.command.ts: 68	Attack Vector

Fixed Issues

Severity	Issue	Source File / Package
	Unpinned Actions Full Length Commit SHA	/build-desktop.yml: 1164
	Unpinned Actions Full Length Commit SHA	/version-auto-bump.yml: 47
	Unpinned Actions Full Length Commit SHA	/build-desktop.yml: 297
	Unpinned Actions Full Length Commit SHA	/build-desktop.yml: 1243
	Client_JQuery_Deprecated_Symbols	/libs/common/src/autofill/services/autofill-settings.service.ts: 128
	Use_Of_Hardcoded_Password	/libs/common/src/tools/send/services/send.service.spec.ts: 495
	Use_Of_Hardcoded_Password	/apps/browser/src/autofill/background/notification.background.spec.ts: 61
	Use_Of_Hardcoded_Password	/apps/browser/src/autofill/enums/notification-queue-message-type.enum.ts: 3
	Use_Of_Hardcoded_Password	/apps/browser/src/autofill/background/notification.background.spec.ts: 61
	Use_Of_Hardcoded_Password	/apps/browser/src/autofill/background/notification.background.spec.ts: 96
	Use_Of_Hardcoded_Password	/apps/cli/src/vault/models/cipher.response.ts: 21
	Use_Of_Hardcoded_Password	/apps/cli/src/vault/models/cipher.response.ts: 21
	Use_Of_Hardcoded_Password	/apps/cli/src/vault/models/cipher.response.ts: 21
	Use_Of_Hardcoded_Password	/apps/cli/src/vault/models/cipher.response.ts: 21
	Use_Of_Hardcoded_Password	/apps/cli/src/vault/models/cipher.response.ts: 21

danielleflinn

This behavior seems to align with the Design request outlined in: PM-3078

@gbubemismith when you get to reviewing this for Vault please reference the internal task which outlines our desired fix for this UX bug.

gbubemismith

@Spitfireap Thanks for fixing this bug. I have one suggestion, and we can get this approved.

gbubemismith · 2024-02-01T21:18:27Z

apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts

-      queryParamsHandling: "merge",
-    });
+  @HostListener("click", ["$event.target"])
+  protected click(target: HTMLSpanElement) {


nitpick: I think the target parameter can be changed to HTMLElement as it is a more general type.

@gbubemismith Change committed ;).
The lint process applied some change on L.37. Do you want to keep them ?

Thanks for jumping on this quickly! I noticed something on a second look: we are shifting our UX to favour clickable cells instead of the entire row. Given this requirement, it looks like removing the HostListener and the HostBinding should meet our aim. Would you mind making those changes? I really appreciate your work on this.

My pleasure and done ! Thanks for your review.
I've deleted L.37 altogether (activatedRouter didn't seem to be used before). I'm not fluent enough in Angular to be sure it wasn't needed (even though I didn't see any regression). I would appreciate it if you could double-check.
On a side note I feel that making the clickable area for the checkbox wider than its visual appearance (as proposed initially) would still be an accessibility improvement...

One last thing: can you fix the conflict on your branch?

CLAassistant · 2024-02-08T17:48:40Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ Spitfireap
❌ gbubemismith
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

gbubemismith · 2024-02-12T18:10:39Z

Hello @Spitfireap, Firstly, thank you for your work on this bug. I moved this to our QA team to test and received feedback on adding this behaviour to collections to keep things consistent. Given the excellent work you have already done, would you mind including this adjustment on the VaultCollectionRowComponent? Please know there's no pressure to take this on if you're not comfortable.

Spitfireap · 2024-02-13T12:09:17Z

@gbubemismith done :).
So no discussion possible about making the clickable area for the checkbox bigger ?

gbubemismith · 2024-02-14T15:04:05Z

@gbubemismith done :). So no discussion possible about making the clickable area for the checkbox bigger ?

Thanks for your quick response. I have started this discussion and am sure this suggestion will be considered.

gbubemismith · 2024-02-14T22:45:38Z

@Spitfireap Our QA team has approved this PR 🚀. Thanks for your work on this.

Spitfireap requested a review from a team as a code owner November 19, 2023 21:31

Spitfireap requested a review from gbubemismith November 19, 2023 21:31

Spitfireap changed the title ~~Increase click area of checkbox on vault~~ Increase click area of checkbox on vault item list Nov 19, 2023

bitwarden-bot changed the title ~~Increase click area of checkbox on vault item list~~ [PM-4871] Increase click area of checkbox on vault item list Nov 19, 2023

bitwarden-bot added the community-pr label Nov 19, 2023

djsmith85 requested a review from a team November 29, 2023 16:17

danielleflinn previously approved these changes Nov 29, 2023

View reviewed changes

djsmith85 linked an issue Dec 1, 2023 that may be closed by this pull request

Web UI Checkbox selection gets wiped with Edit Popup Window #7066

Closed

1 task

gbubemismith self-assigned this Feb 1, 2024

gbubemismith requested changes Feb 1, 2024

View reviewed changes

Spitfireap dismissed danielleflinn’s stale review via 4cb3017 February 3, 2024 12:06

gbubemismith added the needs-qa Marks a PR as requiring QA approval label Feb 7, 2024

Improved UX on vault items

4032a45

Spitfireap force-pushed the master branch from c653c19 to 4032a45 Compare February 8, 2024 08:56

Merge branch 'main' into master

a9e9eed

gbubemismith previously approved these changes Feb 8, 2024

View reviewed changes

gbubemismith mentioned this pull request Feb 8, 2024

[PM-4871] Increase click area of checkbox on vault item list #7860

Closed

danielleflinn changed the title ~~[PM-4871] Increase click area of checkbox on vault item list~~ [PM-4871] Remove clickable row in favor of clickable cell content Feb 9, 2024

gbubemismith self-requested a review February 12, 2024 15:56

Remove clickable row in vault collections

1196f4b

Spitfireap dismissed gbubemismith’s stale review via 1196f4b February 13, 2024 12:08

gbubemismith approved these changes Feb 14, 2024

View reviewed changes

gbubemismith removed the needs-qa Marks a PR as requiring QA approval label Feb 14, 2024

gbubemismith merged commit 973b95f into bitwarden:main Feb 14, 2024
50 of 56 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PM-4871] Remove clickable row in favor of clickable cell content #6911

[PM-4871] Remove clickable row in favor of clickable cell content #6911

Spitfireap commented Nov 19, 2023

bitwarden-bot commented Nov 19, 2023

bitwarden-bot commented Nov 19, 2023 •

edited

Loading

danielleflinn left a comment

gbubemismith left a comment

gbubemismith Feb 1, 2024

Spitfireap Feb 3, 2024

gbubemismith Feb 5, 2024

Spitfireap Feb 6, 2024

gbubemismith Feb 7, 2024

Spitfireap Feb 8, 2024

CLAassistant commented Feb 8, 2024

gbubemismith commented Feb 12, 2024

Spitfireap commented Feb 13, 2024

gbubemismith commented Feb 14, 2024

gbubemismith commented Feb 14, 2024

[PM-4871] Remove clickable row in favor of clickable cell content #6911

[PM-4871] Remove clickable row in favor of clickable cell content #6911

Conversation

Spitfireap commented Nov 19, 2023

Type of change

Objective

Code changes

Screenshots

bitwarden-bot commented Nov 19, 2023

bitwarden-bot commented Nov 19, 2023 • edited Loading

New Issues

Fixed Issues

danielleflinn left a comment

Choose a reason for hiding this comment

gbubemismith left a comment

Choose a reason for hiding this comment

gbubemismith Feb 1, 2024

Choose a reason for hiding this comment

Spitfireap Feb 3, 2024

Choose a reason for hiding this comment

gbubemismith Feb 5, 2024

Choose a reason for hiding this comment

Spitfireap Feb 6, 2024

Choose a reason for hiding this comment

gbubemismith Feb 7, 2024

Choose a reason for hiding this comment

Spitfireap Feb 8, 2024

Choose a reason for hiding this comment

CLAassistant commented Feb 8, 2024

gbubemismith commented Feb 12, 2024

Spitfireap commented Feb 13, 2024

gbubemismith commented Feb 14, 2024

gbubemismith commented Feb 14, 2024

bitwarden-bot commented Nov 19, 2023 •

edited

Loading