Skip to content

[PM-40318] Introduce Web's VaultNextComponet for VFO1 - #22425

Merged
shane-melton merged 10 commits into
mainfrom
vault/pm-40318/web-filter-integration
Aug 17, 2026
Merged

[PM-40318] Introduce Web's VaultNextComponet for VFO1#22425
shane-melton merged 10 commits into
mainfrom
vault/pm-40318/web-filter-integration

Conversation

@shane-melton

@shane-melton shane-melton commented Aug 14, 2026

Copy link
Copy Markdown
Member

🎟️ Tracking

PM-40318

📔 Objective

Introduce the VaultNextComponent that uses the new, shared VaultItemsTableComponent for VFO1.

Router query parameter mapping and legacy query param handling coming in a followup PR.

📸 Screenshots

Screenshot 2026-08-13 at 5 13 46 PM

@shane-melton

Copy link
Copy Markdown
Member Author

@nick-livefront Tagging you since you've just done similar work in Desktop. I have a few minor things to clean up still, but should be ready for an initial review.

@shane-melton shane-melton added ai-review Request a Claude code review t:feature Change Type - Feature Development labels Aug 14, 2026
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the new VaultNextComponent and WebVaultItemActionsService, the VFO1-flagged route swap, and the shared CipherActionService / CipherRowMenuService changes in libs/vault. Previously raised findings on the writable-collection scoping, the shared activeAccount$ → userId stream, and the missing banners/onboarding components were all addressed in 991c947, 8883a7e, and 9477eca. Verified provider availability for the component's new dependency graph, the slot=toolbar projection contract, and that permanentlyDeleteItem exists in all three consuming locale files. One earlier finding was reintroduced by the revert in ad86649.

Code Review Details
  • ⚠️ : Reverting 223b78f reintroduces the dead click for premium users opening attachments on an org item whose organization has no storage allocated
    • libs/vault/src/services/cipher-action.service.ts:237

Comment thread libs/vault/src/services/cipher-row-menu.service.ts Outdated
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.47727% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.11%. Comparing base (25d5c69) to head (ad86649).
⚠️ Report is 25 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...c/app/vault/services/vault-item-actions.service.ts 86.30% 6 Missing and 4 partials ⚠️
...app/vault/individual-vault/vault-routing.module.ts 0.00% 3 Missing ⚠️
...web/src/app/vault/individual-vault/vault.module.ts 0.00% 1 Missing ⚠️
...nents/new-cipher-menu/new-cipher-menu.component.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #22425      +/-   ##
==========================================
+ Coverage   53.93%   54.11%   +0.18%     
==========================================
  Files        4297     4312      +15     
  Lines      136500   137148     +648     
  Branches    21569    21662      +93     
==========================================
+ Hits        73616    74215     +599     
- Misses      57518    57533      +15     
- Partials     5366     5400      +34     

☔ 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.

nick-livefront
nick-livefront previously approved these changes Aug 14, 2026

@nick-livefront nick-livefront left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nothing major to me, looks great!

Comment on lines +59 to +60
this.accountService.activeAccount$.pipe(
getUserId,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⛏️ (non-blocking) This is the second usage of activeAccount$ -> getUserId. Should we make a single reference to active account. Minor benefit of one emission rather than two, Angular might optimize that already 🤷

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Might as well: 8883a7e

Comment thread libs/vault/src/services/cipher-action.service.ts
Comment thread apps/web/src/app/vault/services/vault-item-actions.service.ts
Comment on lines +35 to +38
* Not yet wired: the typed filter adapter that syncs the
* table's chips to the URL, the redirect that rewrites legacy filter query params, and the
* `?itemId=&action=` deep link that opens an item on load. Until the chips are wired there is no
* route to trash or the archive from this page, so both are excluded from the list.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

FYI: I should have a utility method up for review this morning that handles this.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comment thread libs/vault/src/services/cipher-action.service.ts
Comment thread apps/web/src/app/vault/individual-vault/vault-next.component.html
Comment thread apps/web/src/app/vault/individual-vault/vault-next.component.html
@shane-melton
shane-melton marked this pull request as ready for review August 17, 2026 18:03
@shane-melton
shane-melton requested review from a team as code owners August 17, 2026 18:03
nick-livefront
nick-livefront previously approved these changes Aug 17, 2026
@shane-melton
shane-melton removed request for a team and cyprain-okeke August 17, 2026 19:02
@shane-melton

Copy link
Copy Markdown
Member Author

@nick-livefront Removed the billing owned change and added support for the new-cipher-menu component instead of re-creating it in VaultNext.

Comment thread libs/vault/src/services/cipher-action.service.ts
@shane-melton
shane-melton merged commit 0b84b9b into main Aug 17, 2026
166 of 169 checks passed
@shane-melton
shane-melton deleted the vault/pm-40318/web-filter-integration branch August 17, 2026 19:55
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