[PM-7231] Product Switcher within navigation sidebar
Type of change
- [ ] Bug fix
- [x] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
Objective
Following the same logic of the current Product Switcher in the upper right corner, available products and/or additional products should show at the bottom of the navigation sidebar.
Code changes
- product-switcher.service.ts:
- Moved the product switcher logic from
product-switcher-content.component.tsinto a service so it can be shared with the existing product switcher and the new one that exists in the navigation UI. - I did not rename the
bento&otherproperties but I can if there is a desire for a more general term thanbento - Added
navigationUIDetailsto accommodate different verbiage between the navigation product switcher and the current one.
- Moved the product switcher logic from
- navigation-switcher.component.ts:
- Component to display product offerings vertically within the navigation sidebar.
- The active product is always hidden
- toggle-width.component.ts
- This was absolutely positioned before this change because it was the only item at the bottom of the navigation, and probably because it is only shown in the dev environments. I removed the absolutely positioning because it will always display below the product switcher.
- tailwind.config.base.js
- Added
xxsfont size to match the designs for the "switch" hover text
- Added
- Added product switcher to each navigation instance (if I missed one, let me know!). Also made the
navelement a flex container so the product switcher can be pushed to the bottom of the nav.secrets-manager/layout/navigation.component.htmlorganizations/layouts/organization-layout.component.htmllayouts/user-layout.component.htmladmin-console/providers/providers-layout.component.html
Open question
- There is a
PaymentMethodWarningsComponentthat exists below my implementation of the product switcher. I wasn't able to get this to show by manipulating the code in a reasonable amount of time. Will this cause an overlap of sorts?
Screenshots
| More from Bitwarden | Available Products |
|---|---|
Before you submit
- Please add unit tests where it makes sense to do so (encouraged but not required)
- If this change requires a documentation update - notify the documentation team
- If this change has particular deployment requirements - notify the DevOps team
- Ensure that all UI additions follow WCAG AA requirements
Codecov Report
Attention: Patch coverage is 44.95413% with 60 lines in your changes are missing coverage. Please review.
Project coverage is 27.83%. Comparing base (
b4f4818) to head (8d04566).
Additional details and impacted files
@@ Coverage Diff @@
## main #8810 +/- ##
==========================================
+ Coverage 27.79% 27.83% +0.03%
==========================================
Files 2422 2425 +3
Lines 70233 70296 +63
Branches 13096 13099 +3
==========================================
+ Hits 19521 19566 +45
- Misses 49192 49202 +10
- Partials 1520 1528 +8
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Checkmarx One – Scan Summary & Details – 7fefb693-2245-42ad-8c27-b80bc638ad16
No New Or Fixed Issues Found
~~Converting to draft, I realized I missed some acceptance criteria. I need to add some reference events.~~
Edit: reference events are being omitted from the AC
Even though this component is rendered by the sidebar, it actually contains a
bit-bannerwhich is positioned at the top of the page.
@eliykat Thank you for validating and the clarification!
Can you please add a Storybook story for the navigation product switcher? See the existing stories for the current product switcher and the layout, for reference.
Yes! I added new stories for the navigation switcher and fixed the existing product switcher stories which I broke with the added service.
@bitwarden/team-platform-dev should own the layout components, see #8814. I've requested their review, please treat them as a code owner (i.e. don't merge without their approval).
Hey @nick-livefront,
Sorry for the delay. I've discussed the UX with design and we decided to remove the Switch text completely. This should resolve the concerns about it not being clickable. We also discussed some concerns about items changing order as you change product, and the desire is to keep the active product in the nav with a active state similar to general nav items.
Does this seem reasonable to you?
I also had a chat with @willmartian about the duplicated styling and we decided it's fine as it is. There are some future tasks to change how the nav works which would resolve this.
Sorry for the delay. I've discussed the UX with design and we decided to remove the
Switchtext completely. This should resolve the concerns about it not being clickable. We also discussed some concerns about items changing order as you change product, and the desire is to keep the active product in the nav with aactivestate similar to general nav items.Does this seem reasonable to you?
@Hinton I appreciate you tracking down these answers and they both make sense to me! I'll also note them in the ticket.
@Hinton @eliykat @LRNcardozoWDF @Thomas-Avery
I had 3 defects come back, I added each as a commit here. Let me know if an alternate process is preferred!
@Hinton @eliykat @LRNcardozoWDF @Thomas-Avery
I had another defect come back around but to solve I had to alter a component library component, so I pulled that into a separate PR request: ~~https://github.com/bitwarden/clients/pull/8810~~ https://github.com/bitwarden/clients/pull/9128
I assume you mean https://github.com/bitwarden/clients/pull/9128? :)
I assume you mean #9128? :)
Yes I do! Thank you 🙇
@Hinton @eliykat @LRNcardozoWDF @Thomas-Avery
https://github.com/bitwarden/clients/pull/9128 merged into this branch! Ready for review, I appreciate the patience 🙌
@nick-livefront @jtouchstonebw mentioned in chromatic that spacing between icon and text looks a bit smaller than in general.