Skip to content

[PM-40255] Update collection icon to bwi-shared-folder#21996

Open
nick-livefront wants to merge 20 commits into
mainfrom
vault/pm-40255/collection-icon
Open

[PM-40255] Update collection icon to bwi-shared-folder#21996
nick-livefront wants to merge 20 commits into
mainfrom
vault/pm-40255/collection-icon

Conversation

@nick-livefront

@nick-livefront nick-livefront commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

🎟️ Tracking

PM-40255

📔 Objective

Updates usage of bwi-collection and bwi-collection-shared to be bwi-shared-folder globally.

  • All changes are behind the vfo-foundation feature flag
  • Where possible I updated the tech debt of using <i> to <bit-icon>

NOTE: There are still hardcoded examples in storybook. These are accounted for in PM-40765 which will be handled post launch work

📸 Screenshots

Not every instance was screenshot.

@bitwarden/team-auth-dev - I wasn't getting any organization ciphers in the emergency view to return from the endpoint so I don't have a screenshot for you. Is that expected? Possibly a unique scenario?

Vault
Web Extension Desktop Edit Menu
Screenshot 2026-07-22 at 1 14 29 PM Screenshot 2026-07-22 at 1 15 01 PM Screenshot 2026-07-22 at 1 15 28 PM Screenshot 2026-07-22 at 1 15 35 PM
Admin Console
AC Collections Group Access
Screenshot 2026-07-22 at 1 13 53 PM Screenshot 2026-07-22 at 1 14 17 PM Screenshot 2026-07-22 at 1 58 35 PM
Autofill
Fido2
Screenshot 2026-07-22 at 1 01 39 PM
DIRT
Exposed Passwords Weak Passwords
Screenshot 2026-07-22 at 1 27 41 PM Screenshot 2026-07-22 at 1 28 00 PM
Tools
Import
Screenshot 2026-07-22 at 1 14 54 PM

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed this feature-flagged UI refinement that swaps collection icons to bwi-shared-folder across browser, desktop, and web, routed through Vfo1TerminologyService.iconClass() and the Vfo1IconPipe behind the VFO1Foundation flag. Verified DI wiring (desktop safeProvider deps, web derived VaultFilterService, and all spec mocks), the documented circular-dependency workaround in import.component.ts, and that flag-off behavior is preserved. The two prior review threads (broken batch-action test, bwi-collection icon change) have both been addressed by the author.

Code Review Details

No new findings. The change is mechanical and consistently gated behind the feature flag; the mapping is applied exactly once at each call site, and the BitwardenIcon typing tightens the previous string signatures.

Prior threads resolved:

  • The broken vault-batch-action.component.spec.ts assertion was fixed by reverting to bwi-collection (7f72c20).
  • The accidental flag-off icon change was reverted, with bwi-collection added to the flag-on map so only flag-on behavior changes (0d9bf30).

@nick-livefront nick-livefront added the ai-review Request a Claude code review label Jul 22, 2026
@nick-livefront nick-livefront added the t:feature Change Type - Feature Development label Jul 22, 2026
Comment thread libs/vault/src/components/vault-batch-bar/vault-batch-action.component.ts Outdated
Comment thread libs/vault/src/components/vault-batch-bar/vault-batch-action.component.spec.ts Outdated
@bw-ghapp

bw-ghapp Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Changes in this PR impact the Autofill experience of the browser client

BIT has tested the core experience with these changes and the feature flag configuration used by vault.bitwarden.com.

✅ Fortunately, these BIT tests have passed! 🎉

@bw-ghapp

bw-ghapp Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Changes in this PR impact the Autofill experience of the browser client

BIT has tested the core experience with these changes and all feature flags disabled.

✅ Fortunately, these BIT tests have passed! 🎉

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 63.26531% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.28%. Comparing base (1c3bb59) to head (7f72c20).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
libs/importer/src/components/import.component.ts 0.00% 8 Missing ⚠️
...t-v3/vault-items/vault-collection-row.component.ts 0.00% 2 Missing ⚠️
...her-view/item-details/item-details-v2.component.ts 60.00% 2 Missing ⚠️
...autofill/popup/fido2/fido2-cipher-row.component.ts 0.00% 1 Missing ⚠️
...popup/services/vault-popup-list-filters.service.ts 75.00% 0 Missing and 1 partial ⚠️
...in-console/organizations/members/members.module.ts 0.00% 1 Missing ⚠️
...admin-console/organizations/organization.module.ts 0.00% 1 Missing ⚠️
...nents/access-selector/access-selector.component.ts 66.66% 1 Missing ⚠️
...ncy-access/view/emergency-access-view.component.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #21996      +/-   ##
==========================================
- Coverage   52.44%   52.28%   -0.16%     
==========================================
  Files        4130     4158      +28     
  Lines      130572   131564     +992     
  Branches    20494    20666     +172     
==========================================
+ Hits        68475    68793     +318     
- Misses      57045    57707     +662     
- Partials     5052     5064      +12     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jprusik jprusik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved for Autofill concerns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants