[PM-8603] Browser - Sticky Search/Filters

nick-livefront commented 3 days ago

🎟️ Tracking

PM-8603

📔 Objective

Make the Search & Filters sticky
- I switched around some padding/margin to split the spacing between the stuck element and the cipher list components

Bottom border on scroll

I created the StickyContainerComponent to handle the bottom border that should be shown on scroll. I wanted to separate this UI logic from any other business logic so it is a fairly small component.
I ended up with an okay approach of tying the scrollable container via a static id and listening for scroll events. In this case the window isn't the element that is scrolled.

Other attempts

Showing a border/shadow on scroll isn't necessarily new in my toolbox but this one gave me some trouble.

I usually manage this with overlapping pseudo elements. For this implementation I would need to know the css top value so that border can be "stuck" to the bottom of the search/filters. This was proving difficult because the filters can hide/show so I would have had to consistently check the height of that element. I stopped there, that didn't feel sustainable.
I also made an attempt at placing overlapping dummy divs into the DOM but I was creating dependencies on the negative margin value and once again the top value.

📸 Screenshots

https://github.com/bitwarden/clients/assets/125900171/2405951e-544a-4f27-84aa-c2fc970611bc

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

codecov[bot] commented 3 days ago

Codecov Report

Attention: Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.

Project coverage is 29.34%. Comparing base (1826403) to head (2db1b0e). Report is 12 commits behind head on main.

Files	Patch %	Lines
.../src/platform/popup/layout/popup-page.component.ts	66.66%	1 Missing :warning:
...vault/popup/components/vault/vault-v2.component.ts	0.00%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #9830 +/- ## ========================================== + Coverage 29.28% 29.34% +0.06% ========================================== Files 2532 2528 -4 Lines 73822 73773 -49 Branches 13781 13768 -13 ========================================== + Hits 21620 21651 +31 + Misses 50580 50500 -80 Partials 1622 1622 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 3 days ago

Checkmarx One – Scan Summary & Details – 7094dd8a-29a7-4330-883d-97cacfbe31c3

New Issues

Severity	Issue	Source File / Package	Checkmarx Insight
	Client_Use_Of_Iframe_Without_Sandbox	/apps/web/src/connectors/duo.ts: 8	Attack Vector

nick-livefront commented 3 days ago

After discussing with Danielle in PM-8602 about a scrollbar issue. I'm going to refactor how this works. Closing this PR in favor of a revamped solution.

bitwarden / clients