[CL-473] Adjust popup page max width and scroll containers (#14853)

apps/browser/src/platform/popup/layout/popup-layout.mdx

@@ -54,6 +54,9 @@ page looks nice when the extension is popped out.
     `false`.
 - `loadingText`
   - Custom text to be applied to the loading element for screenreaders only. Defaults to "Loading".
+- `disablePadding`
+  - When `true`, disables the padding of the scrollable region inside of `main`. You will need to
+    add your own padding to the element you place inside of this area.
 
 Basic usage example:
 
@@ -169,6 +172,22 @@ When the browser extension is popped out, the "popout" button should not be pass
 
 <Canvas of={stories.PoppedOut} />
 
+## With Virtual Scroll
+
+If you are using a virtual scrolling container inside of the popup page, you'll want to apply the
+`bitScrollLayout` directive to the `cdk-virtual-scroll-viewport` element. This tells the virtual
+scroll viewport to use the popup page's scroll layout div as the scrolling container.
+
+See the code in the example below.
+
+<Canvas of={stories.WithVirtualScrollChild} />
+
+### Known Virtual Scroll Issues
+
+See [Virtual Scrolling](?path=/docs/documentation-virtual-scrolling--docs#known-footgun) for more
+information about how to structure virtual scrolling containers with layout components and avoid a
+known issue with template construction.
+
 # Other stories
 
 ## Centered Content

apps/browser/src/platform/popup/layout/popup-layout.stories.ts

@@ -1,5 +1,4 @@
-// FIXME: Update this file to be type safe and remove this and next line
+import { ScrollingModule } from "@angular/cdk/scrolling";
-// @ts-strict-ignore
 import { CommonModule } from "@angular/common";
 import { Component, importProvidersFrom } from "@angular/core";
 import { RouterModule } from "@angular/router";
@@ -20,6 +19,7 @@ import {
   NoItemsModule,
   SearchModule,
   SectionComponent,
+  ScrollLayoutDirective,
 } from "@bitwarden/components";
 
 import { PopupRouterCacheService } from "../view-cache/popup-router-cache.service";
@@ -39,6 +39,17 @@ import { PopupTabNavigationComponent } from "./popup-tab-navigation.component";
 })
 class ExtensionContainerComponent {}
 
+@Component({
+  selector: "extension-popped-container",
+  template: `
+    <div class="tw-h-[640px] tw-w-[900px] tw-border tw-border-solid tw-border-secondary-300">
+      <ng-content></ng-content>
+    </div>
+  `,
+  standalone: true,
+})
+class ExtensionPoppedContainerComponent {}
+
 @Component({
   selector: "vault-placeholder",
   template: /*html*/ `
@@ -295,6 +306,7 @@ export default {
   decorators: [
     moduleMetadata({
       imports: [
+        ScrollLayoutDirective,
         PopupTabNavigationComponent,
         PopupHeaderComponent,
         PopupPageComponent,
@@ -302,6 +314,7 @@ export default {
         CommonModule,
         RouterModule,
         ExtensionContainerComponent,
+        ExtensionPoppedContainerComponent,
         MockBannerComponent,
         MockSearchComponent,
         MockVaultSubpageComponent,
@@ -312,6 +325,11 @@ export default {
         MockVaultPagePoppedComponent,
         NoItemsModule,
         VaultComponent,
+        ScrollingModule,
+        ItemModule,
+        SectionComponent,
+        IconButtonModule,
+        BadgeModule,
       ],
       providers: [
         {
@@ -495,7 +513,21 @@ export const CompactMode: Story = {
       const compact = canvasEl.querySelector(
         `#${containerId} [data-testid=popup-layout-scroll-region]`,
       );
+
+      if (!compact) {
+        // eslint-disable-next-line
+        console.error(`#${containerId} [data-testid=popup-layout-scroll-region] not found`);
+        return;
+      }
+
       const label = canvasEl.querySelector(`#${containerId} .example-label`);
+
+      if (!label) {
+        // eslint-disable-next-line
+        console.error(`#${containerId} .example-label not found`);
+        return;
+      }
+
       const percentVisible =
         100 -
         Math.round((100 * (compact.scrollHeight - compact.clientHeight)) / compact.scrollHeight);
@@ -510,9 +542,9 @@ export const PoppedOut: Story = {
   render: (args) => ({
     props: args,
     template: /* HTML */ `
-      <div class="tw-h-[640px] tw-w-[900px] tw-border tw-border-solid tw-border-secondary-300">
+      <extension-popped-container>
         <mock-vault-page-popped></mock-vault-page-popped>
-      </div>
+      </extension-popped-container>
     `,
   }),
 };
@@ -560,10 +592,9 @@ export const TransparentHeader: Story = {
     template: /* HTML */ `
       <extension-container>
         <popup-page>
-          <popup-header slot="header" background="alt"
+          <popup-header slot="header" background="alt">
-            ><span class="tw-italic tw-text-main">🤠 Custom Content</span></popup-header
+            <span class="tw-italic tw-text-main">🤠 Custom Content</span>
-          >
+          </popup-header>
-
           <vault-placeholder></vault-placeholder>
         </popup-page>
       </extension-container>
@@ -608,3 +639,56 @@ export const WidthOptions: Story = {
     `,
   }),
 };
+
+export const WithVirtualScrollChild: Story = {
+  render: (args) => ({
+    props: { ...args, data: Array.from(Array(20).keys()) },
+    template: /* HTML */ `
+      <extension-popped-container>
+        <popup-page>
+          <popup-header slot="header" pageTitle="Test"> </popup-header>
+          <mock-search slot="above-scroll-area"></mock-search>
+          <bit-section>
+            @defer (on immediate) {
+            <bit-item-group aria-label="Mock Vault Items">
+              <cdk-virtual-scroll-viewport itemSize="61" bitScrollLayout>
+                <bit-item *cdkVirtualFor="let item of data; index as i">
+                  <button type="button" bit-item-content>
+                    <i
+                      slot="start"
+                      class="bwi bwi-globe tw-text-3xl tw-text-muted"
+                      aria-hidden="true"
+                    ></i>
+                    {{ i }} of {{ data.length - 1 }}
+                    <span slot="secondary">Bar</span>
+                  </button>
+
+                  <ng-container slot="end">
+                    <bit-item-action>
+                      <button type="button" bitBadge variant="primary">Fill</button>
+                    </bit-item-action>
+                    <bit-item-action>
+                      <button
+                        type="button"
+                        bitIconButton="bwi-clone"
+                        aria-label="Copy item"
+                      ></button>
+                    </bit-item-action>
+                    <bit-item-action>
+                      <button
+                        type="button"
+                        bitIconButton="bwi-ellipsis-v"
+                        aria-label="More options"
+                      ></button>
+                    </bit-item-action>
+                  </ng-container>
+                </bit-item>
+              </cdk-virtual-scroll-viewport>
+            </bit-item-group>
+            }
+          </bit-section>
+        </popup-page>
+      </extension-popped-container>
+    `,
+  }),
+};

apps/browser/src/platform/popup/layout/popup-page.component.html

@@ -1,30 +1,40 @@
 <ng-content select="[slot=header]"></ng-content>
 <main class="tw-flex-1 tw-overflow-hidden tw-flex tw-flex-col tw-relative tw-bg-background-alt">
   <ng-content select="[slot=full-width-notice]"></ng-content>
+  <!-- 
+    x padding on this container is designed to always be a minimum of 0.75rem (equivalent to tailwind's tw-px-3), or 0.5rem (equivalent
+    to tailwind's tw-px-2) in compact mode, but stretch to fill the remainder of the container when the content reaches a maximum of
+    640px in width (equivalent to tailwind's `sm` breakpoint)
+  -->
   <div
     #nonScrollable
-    class="tw-transition-colors tw-duration-200 tw-border-0 tw-border-b tw-border-solid tw-p-3 bit-compact:tw-p-2"
+    class="tw-transition-colors tw-duration-200 tw-border-0 tw-border-b tw-border-solid tw-py-3 bit-compact:tw-py-2 tw-px-[max(0.75rem,calc((100%-(var(--tw-sm-breakpoint)))/2))] bit-compact:tw-px-[max(0.5rem,calc((100%-(var(--tw-sm-breakpoint)))/2))]"
     [ngClass]="{
-      'tw-invisible !tw-p-0': loading || nonScrollable.childElementCount === 0,
+      'tw-invisible !tw-p-0 !tw-border-none': loading || nonScrollable.childElementCount === 0,
       'tw-border-secondary-300': scrolled(),
       'tw-border-transparent': !scrolled(),
     }"
   >
     <ng-content select="[slot=above-scroll-area]"></ng-content>
   </div>
+  <!-- 
+    x padding on this container is designed to always be a minimum of 0.75rem (equivalent to tailwind's tw-px-3), or 0.5rem (equivalent
+    to tailwind's tw-px-2) in compact mode, but stretch to fill the remainder of the container when the content reaches a maximum of
+    640px in width (equivalent to tailwind's `sm` breakpoint)
+  -->
   <div
-    class="tw-max-w-screen-sm tw-mx-auto tw-overflow-y-auto tw-flex tw-flex-col tw-size-full tw-styled-scrollbar"
+    class="tw-overflow-y-auto tw-size-full tw-styled-scrollbar"
     data-testid="popup-layout-scroll-region"
     (scroll)="handleScroll($event)"
-    [ngClass]="{ 'tw-invisible': loading }"
+    [ngClass]="{
-  >
+      'tw-invisible': loading,
-    <div
+      'tw-py-3 bit-compact:tw-py-2 tw-px-[max(0.75rem,calc((100%-(var(--tw-sm-breakpoint)))/2))] bit-compact:tw-px-[max(0.5rem,calc((100%-(var(--tw-sm-breakpoint)))/2))]':
-      class="tw-max-w-screen-sm tw-mx-auto tw-flex-1 tw-flex tw-flex-col tw-w-full"
+        !disablePadding,
-      [ngClass]="{ 'tw-p-3 bit-compact:tw-p-2': !disablePadding }"
+    }"
+    bitScrollLayoutHost
   >
     <ng-content></ng-content>
   </div>
-  </div>
   <span
     class="tw-absolute tw-inset-0 tw-flex tw-items-center tw-justify-center tw-text-main"
     [ngClass]="{ 'tw-invisible': !loading }"

apps/browser/src/platform/popup/layout/popup-page.component.ts

@@ -2,6 +2,7 @@ import { CommonModule } from "@angular/common";
 import { booleanAttribute, Component, inject, Input, signal } from "@angular/core";
 
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { ScrollLayoutHostDirective } from "@bitwarden/components";
 
 @Component({
   selector: "popup-page",
@@ -9,7 +10,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic
   host: {
     class: "tw-h-full tw-flex tw-flex-col tw-overflow-y-hidden",
   },
-  imports: [CommonModule],
+  imports: [CommonModule, ScrollLayoutHostDirective],
 })
 export class PopupPageComponent {
   protected i18nService = inject(I18nService);

apps/browser/src/popup/scss/base.scss

@@ -381,14 +381,6 @@ app-root {
   }
 }
 
-// Adds padding on each side of the content if opened in a tab
-@media only screen and (min-width: 601px) {
-  header,
-  main {
-    padding: 0 calc((100% - 500px) / 2);
-  }
-}
-
 main:not(popup-page main) {
   position: absolute;
   top: 44px;

apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html

@@ -89,10 +89,7 @@
         </h3>
       </ng-container>
 
-      <cdk-virtual-scroll-viewport
+      <cdk-virtual-scroll-viewport [itemSize]="itemHeight$ | async" bitScrollLayout>
-        [itemSize]="itemHeight$ | async"
-        class="tw-overflow-visible [&>.cdk-virtual-scroll-content-wrapper]:[contain:layout_style]"
-      >
         <bit-item *cdkVirtualFor="let cipher of group.ciphers">
           <button
             bit-item-content

apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts

@@ -42,6 +42,7 @@ import {
   SectionComponent,
   SectionHeaderComponent,
   TypographyModule,
+  ScrollLayoutDirective,
 } from "@bitwarden/components";
 import {
   DecryptionFailureDialogComponent,
@@ -74,6 +75,7 @@ import { ItemMoreOptionsComponent } from "../item-more-options/item-more-options
     ScrollingModule,
     DisclosureComponent,
     DisclosureTriggerForDirective,
+    ScrollLayoutDirective,
   ],
   selector: "app-vault-list-items-container",
   templateUrl: "vault-list-items-container.component.html",

apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html

@@ -1,4 +1,4 @@
-<popup-page [loading]="loading$ | async" disablePadding>
+<popup-page [loading]="loading$ | async">
   <popup-header slot="header" [pageTitle]="'vault' | i18n">
     <ng-container slot="end">
       <app-new-item-dropdown [initialValues]="newItemItemValues$ | async"></app-new-item-dropdown>
@@ -84,11 +84,7 @@
       </div>
     </div>
 
-    <div
+    <ng-container *ngIf="vaultState === null">
-      *ngIf="vaultState === null"
-      cdkVirtualScrollingElement
-      class="tw-h-full tw-p-3 bit-compact:tw-p-2 tw-styled-scrollbar"
-    >
       <app-autofill-vault-list-items></app-autofill-vault-list-items>
       <app-vault-list-items-container
         [title]="'favorites' | i18n"
@@ -103,6 +99,6 @@
         disableSectionMargin
         collapsibleKey="allItems"
       ></app-vault-list-items-container>
-    </div>
+    </ng-container>
   </ng-container>
 </popup-page>

libs/components/src/stories/virtual-scrolling.mdx

@@ -16,7 +16,7 @@ We export a similar directive, `bitScrollLayout`, that integrates with `bit-layo
 and should be used instead of `scrollWindow`.
 
 ```html
-<!-- Descendant of bit-layout -->
+<!-- Descendant of bit-layout or popup-page -->
 <cdk-virtual-scroll-viewport bitScrollLayout>
   <!-- virtual scroll implementation here -->
 </cdk-virtual-scroll-viewport>
@@ -27,7 +27,10 @@ and should be used instead of `scrollWindow`.
 Due to the initialization order of Angular components and their templates, `bitScrollLayout` will
 error if it is used _in the same template_ as the layout component:
 
+With `bit-layout`:
+
 ```html
+<!-- Will cause `bitScrollLayout` to error -->
 <bit-layout>
   <cdk-virtual-scroll-viewport bitScrollLayout>
     <!-- virtual scroll implementation here -->
@@ -35,20 +38,43 @@ error if it is used _in the same template_ as the layout component:
 </bit-layout>
 ```
 
+With `popup-page`:
+
+```html
+<!-- Will cause `bitScrollLayout` to error -->
+<popup-page>
+  <cdk-virtual-scroll-viewport bitScrollLayout>
+    <!-- virtual scroll implementation here -->
+  </cdk-virtual-scroll-viewport>
+</popup-page>
+```
+
 In this particular composition, the child content gets constructed before the template of
-`bit-layout` and thus has no scroll container to reference. Workarounds include:
+`bit-layout` (or `popup-page`) and thus has no scroll container to reference. Workarounds include:
 
 1. Wrap the child in another component. (This tends to happen by default when the layout is
    integrated with a `router-outlet`.)
 
+With `bit-layout`:
+
 ```html
 <bit-layout>
   <component-that-contains-bitScrollLayout></component-that-contains-bitScrollLayout>
 </bit-layout>
 ```
 
+With `popup-page`:
+
+```html
+<popup-page>
+  <component-that-contains-bitScrollLayout></component-that-contains-bitScrollLayout>
+</popup-page>
+```
+
 2. Use a `defer` block.
 
+With `bit-layout`:
+
 ```html
 <bit-layout>
   @defer (on immediate) {
@@ -58,3 +84,15 @@ In this particular composition, the child content gets constructed before the te
   }
 </bit-layout>
 ```
+
+With `popup-page`:
+
+```html
+<popup-page>
+  @defer (on immediate) {
+    <cdk-virtual-scroll-viewport bitScrollLayout>
+      <!-- virtual scroll implementation here -->
+    </div>
+  }
+</popup-page>
+```

libs/components/src/tw-theme.css

@@ -58,6 +58,8 @@
   --color-marketing-logo: 23 93 220;
 
   --tw-ring-offset-color: #ffffff;
+
+  --tw-sm-breakpoint: 640px;
 }
 
 .theme_light {