From 4f060d88fa9e7f1d9e387bd27f08d602ccc2aaef Mon Sep 17 00:00:00 2001 From: Victoria League Date: Fri, 27 Dec 2024 15:42:35 -0500 Subject: [PATCH] [CL-509][PM-16190] Avoid double scrollbar appearing when default zoom is >100% (#12427) --- .../src/platform/browser/browser-api.ts | 15 +++++++++ .../src/platform/popup/browser-popup-utils.ts | 18 ++++++++++- .../popup/layout/popup-page.component.ts | 2 +- ...width.service.ts => popup-size.service.ts} | 32 +++++++++++++++---- apps/browser/src/popup/app.component.ts | 3 -- apps/browser/src/popup/main.ts | 4 +-- apps/browser/src/popup/scss/base.scss | 18 ++++++----- .../src/popup/services/init.service.ts | 13 +++----- .../settings/appearance-v2.component.spec.ts | 6 ++-- .../popup/settings/appearance-v2.component.ts | 10 +++--- 10 files changed, 84 insertions(+), 37 deletions(-) rename apps/browser/src/platform/popup/layout/{popup-width.service.ts => popup-size.service.ts} (60%) diff --git a/apps/browser/src/platform/browser/browser-api.ts b/apps/browser/src/platform/browser/browser-api.ts index 05ea3c84e9e..dc71d50d4b1 100644 --- a/apps/browser/src/platform/browser/browser-api.ts +++ b/apps/browser/src/platform/browser/browser-api.ts @@ -165,6 +165,21 @@ export class BrowserApi { }); } + /** + * Fetch the currently open browser tab + */ + static async getCurrentTab(): Promise | null { + if (BrowserApi.isManifestVersion(3)) { + return await chrome.tabs.getCurrent(); + } + + return new Promise((resolve) => + chrome.tabs.getCurrent((tab) => { + resolve(tab); + }), + ); + } + static async tabsQuery(options: chrome.tabs.QueryInfo): Promise { return new Promise((resolve) => { chrome.tabs.query(options, (tabs) => { diff --git a/apps/browser/src/platform/popup/browser-popup-utils.ts b/apps/browser/src/platform/popup/browser-popup-utils.ts index 35fd2361014..33a1ff4016d 100644 --- a/apps/browser/src/platform/popup/browser-popup-utils.ts +++ b/apps/browser/src/platform/popup/browser-popup-utils.ts @@ -3,7 +3,7 @@ import { BrowserApi } from "../browser/browser-api"; import { ScrollOptions } from "./abstractions/browser-popup-utils.abstractions"; -import { PopupWidthOptions } from "./layout/popup-width.service"; +import { PopupWidthOptions } from "./layout/popup-size.service"; class BrowserPopupUtils { /** @@ -24,6 +24,22 @@ class BrowserPopupUtils { return BrowserPopupUtils.urlContainsSearchParams(win, "uilocation", "popout"); } + /** + * Check if the current popup view is open inside of the current browser tab + * (it is possible in Chrome to open the extension in a tab) + */ + static async isInTab() { + const tabId = (await BrowserApi.getCurrentTab())?.id; + + if (tabId === undefined || tabId === null) { + return false; + } + + const result = BrowserApi.getExtensionViews({ tabId, type: "tab" }); + + return result.length > 0; + } + /** * Identifies if the popup is within the single action popout. * diff --git a/apps/browser/src/platform/popup/layout/popup-page.component.ts b/apps/browser/src/platform/popup/layout/popup-page.component.ts index 7b4665040fb..ca019c16bd7 100644 --- a/apps/browser/src/platform/popup/layout/popup-page.component.ts +++ b/apps/browser/src/platform/popup/layout/popup-page.component.ts @@ -8,7 +8,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic templateUrl: "popup-page.component.html", standalone: true, host: { - class: "tw-h-full tw-flex tw-flex-col tw-flex-1 tw-overflow-y-hidden", + class: "tw-h-full tw-flex tw-flex-col tw-overflow-y-hidden", }, imports: [CommonModule], }) diff --git a/apps/browser/src/platform/popup/layout/popup-width.service.ts b/apps/browser/src/platform/popup/layout/popup-size.service.ts similarity index 60% rename from apps/browser/src/platform/popup/layout/popup-width.service.ts rename to apps/browser/src/platform/popup/layout/popup-size.service.ts index d8706dbcbcf..3ae9a633cab 100644 --- a/apps/browser/src/platform/popup/layout/popup-width.service.ts +++ b/apps/browser/src/platform/popup/layout/popup-size.service.ts @@ -7,6 +7,8 @@ import { POPUP_STYLE_DISK, } from "@bitwarden/common/platform/state"; +import BrowserPopupUtils from "../browser-popup-utils"; + /** * * Value represents width in pixels @@ -25,10 +27,12 @@ const POPUP_WIDTH_KEY_DEF = new KeyDefinition(POPUP_STYLE_DISK }); /** - * Updates the extension popup width based on a user setting + * Handles sizing the popup based on available width/height, which can be affected by + * user default zoom level. + * Updates the extension popup width based on a user setting. **/ @Injectable({ providedIn: "root" }) -export class PopupWidthService { +export class PopupSizeService { private static readonly LocalStorageKey = "bw-popup-width"; private readonly state = inject(GlobalStateProvider).get(POPUP_WIDTH_KEY_DEF); @@ -41,15 +45,31 @@ export class PopupWidthService { } /** Begin listening for state changes */ - init() { + async init() { this.width$.subscribe((width: PopupWidthOption) => { - PopupWidthService.setStyle(width); - localStorage.setItem(PopupWidthService.LocalStorageKey, width); + PopupSizeService.setStyle(width); + localStorage.setItem(PopupSizeService.LocalStorageKey, width); }); + + const isInChromeTab = await BrowserPopupUtils.isInTab(); + + if (!BrowserPopupUtils.inPopup(window) || isInChromeTab) { + window.document.body.classList.add("body-full"); + } else if (window.innerHeight < 400) { + window.document.body.classList.add("body-xxs"); + } else if (window.innerHeight < 500) { + window.document.body.classList.add("body-xs"); + } else if (window.innerHeight < 600) { + window.document.body.classList.add("body-sm"); + } } private static setStyle(width: PopupWidthOption) { + if (!BrowserPopupUtils.inPopup(window)) { + return; + } const pxWidth = PopupWidthOptions[width] ?? PopupWidthOptions.default; + document.body.style.minWidth = `${pxWidth}px`; } @@ -57,7 +77,7 @@ export class PopupWidthService { * To keep the popup size from flickering on bootstrap, we store the width in `localStorage` so we can quickly & synchronously reference it. **/ static initBodyWidthFromLocalStorage() { - const storedValue = localStorage.getItem(PopupWidthService.LocalStorageKey); + const storedValue = localStorage.getItem(PopupSizeService.LocalStorageKey); this.setStyle(storedValue as any); } } diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 3382e24689a..8e51152be2e 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -23,7 +23,6 @@ import { } from "@bitwarden/components"; import { PopupCompactModeService } from "../platform/popup/layout/popup-compact-mode.service"; -import { PopupWidthService } from "../platform/popup/layout/popup-width.service"; import { PopupViewCacheService } from "../platform/popup/view-cache/popup-view-cache.service"; import { initPopupClosedListener } from "../platform/services/popup-view-cache-background.service"; import { VaultBrowserStateService } from "../vault/services/vault-browser-state.service"; @@ -42,7 +41,6 @@ import { DesktopSyncVerificationDialogComponent } from "./components/desktop-syn export class AppComponent implements OnInit, OnDestroy { private viewCacheService = inject(PopupViewCacheService); private compactModeService = inject(PopupCompactModeService); - private widthService = inject(PopupWidthService); private lastActivity: Date; private activeUserId: UserId; @@ -73,7 +71,6 @@ export class AppComponent implements OnInit, OnDestroy { await this.viewCacheService.init(); this.compactModeService.init(); - this.widthService.init(); // Component states must not persist between closing and reopening the popup, otherwise they become dead objects // Clear them aggressively to make sure this doesn't occur diff --git a/apps/browser/src/popup/main.ts b/apps/browser/src/popup/main.ts index db634ea2e2c..8ffe0743bf6 100644 --- a/apps/browser/src/popup/main.ts +++ b/apps/browser/src/popup/main.ts @@ -1,7 +1,7 @@ import { enableProdMode } from "@angular/core"; import { platformBrowserDynamic } from "@angular/platform-browser-dynamic"; -import { PopupWidthService } from "../platform/popup/layout/popup-width.service"; +import { PopupSizeService } from "../platform/popup/layout/popup-size.service"; import { BrowserPlatformUtilsService } from "../platform/services/platform-utils/browser-platform-utils.service"; require("./scss/popup.scss"); @@ -10,7 +10,7 @@ require("./scss/tailwind.css"); import { AppModule } from "./app.module"; // We put these first to minimize the delay in window changing. -PopupWidthService.initBodyWidthFromLocalStorage(); +PopupSizeService.initBodyWidthFromLocalStorage(); // Should be removed once we deprecate support for Safari 16.0 and older. See Jira ticket [PM-1861] if (BrowserPlatformUtilsService.shouldApplySafariHeightFix(window)) { document.documentElement.classList.add("safari_height_fix"); diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index 49847dd7778..554396fa038 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -19,8 +19,8 @@ body { } body { - min-width: 380px; - height: 600px !important; + width: 380px; + height: 600px; position: relative; min-height: 100vh; overflow: hidden; @@ -33,18 +33,20 @@ body { } &.body-sm { - width: 375px !important; - height: 500px !important; + height: 500px; } &.body-xs { - width: 375px !important; - height: 300px !important; + height: 400px; + } + + &.body-xxs { + height: 300px; } &.body-full { - width: 100% !important; - height: 100% !important; + width: 100%; + height: 100%; } } diff --git a/apps/browser/src/popup/services/init.service.ts b/apps/browser/src/popup/services/init.service.ts index 9e6471eaf28..24661438495 100644 --- a/apps/browser/src/popup/services/init.service.ts +++ b/apps/browser/src/popup/services/init.service.ts @@ -1,5 +1,5 @@ import { DOCUMENT } from "@angular/common"; -import { Inject, Injectable } from "@angular/core"; +import { inject, Inject, Injectable } from "@angular/core"; import { AbstractThemingService } from "@bitwarden/angular/platform/services/theming/theming.service.abstraction"; import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service"; @@ -10,8 +10,11 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.serv import { BrowserApi } from "../../platform/browser/browser-api"; import BrowserPopupUtils from "../../platform/popup/browser-popup-utils"; +import { PopupSizeService } from "../../platform/popup/layout/popup-size.service"; @Injectable() export class InitService { + private sizeService = inject(PopupSizeService); + constructor( private platformUtilsService: PlatformUtilsService, private i18nService: I18nService, @@ -28,13 +31,7 @@ export class InitService { await this.i18nService.init(); this.twoFactorService.init(); - if (!BrowserPopupUtils.inPopup(window)) { - window.document.body.classList.add("body-full"); - } else if (window.screen.availHeight < 600) { - window.document.body.classList.add("body-xs"); - } else if (window.screen.availHeight <= 800) { - window.document.body.classList.add("body-sm"); - } + await this.sizeService.init(); const htmlEl = window.document.documentElement; this.themingService.applyThemeChangesTo(this.document); diff --git a/apps/browser/src/vault/popup/settings/appearance-v2.component.spec.ts b/apps/browser/src/vault/popup/settings/appearance-v2.component.spec.ts index 38fbe659083..bca83a2fba0 100644 --- a/apps/browser/src/vault/popup/settings/appearance-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/settings/appearance-v2.component.spec.ts @@ -16,7 +16,7 @@ import { ThemeStateService } from "@bitwarden/common/platform/theming/theme-stat import { PopupCompactModeService } from "../../../platform/popup/layout/popup-compact-mode.service"; import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component"; import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component"; -import { PopupWidthService } from "../../../platform/popup/layout/popup-width.service"; +import { PopupSizeService } from "../../../platform/popup/layout/popup-size.service"; import { VaultPopupCopyButtonsService } from "../services/vault-popup-copy-buttons.service"; import { AppearanceV2Component } from "./appearance-v2.component"; @@ -55,7 +55,7 @@ describe("AppearanceV2Component", () => { const setEnableCompactMode = jest.fn().mockResolvedValue(undefined); const setShowQuickCopyActions = jest.fn().mockResolvedValue(undefined); - const mockWidthService: Partial = { + const mockWidthService: Partial = { width$: new BehaviorSubject("default"), setWidth: jest.fn().mockResolvedValue(undefined), }; @@ -95,7 +95,7 @@ describe("AppearanceV2Component", () => { } as Partial, }, { - provide: PopupWidthService, + provide: PopupSizeService, useValue: mockWidthService, }, ], diff --git a/apps/browser/src/vault/popup/settings/appearance-v2.component.ts b/apps/browser/src/vault/popup/settings/appearance-v2.component.ts index c9e616b8e8e..7f300a508a6 100644 --- a/apps/browser/src/vault/popup/settings/appearance-v2.component.ts +++ b/apps/browser/src/vault/popup/settings/appearance-v2.component.ts @@ -25,8 +25,8 @@ import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-heade import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component"; import { PopupWidthOption, - PopupWidthService, -} from "../../../platform/popup/layout/popup-width.service"; + PopupSizeService, +} from "../../../platform/popup/layout/popup-size.service"; import { VaultPopupCopyButtonsService } from "../services/vault-popup-copy-buttons.service"; @Component({ @@ -49,7 +49,7 @@ import { VaultPopupCopyButtonsService } from "../services/vault-popup-copy-butto export class AppearanceV2Component implements OnInit { private compactModeService = inject(PopupCompactModeService); private copyButtonsService = inject(VaultPopupCopyButtonsService); - private popupWidthService = inject(PopupWidthService); + private popupSizeService = inject(PopupSizeService); private i18nService = inject(I18nService); appearanceForm = this.formBuilder.group({ @@ -103,7 +103,7 @@ export class AppearanceV2Component implements OnInit { const showQuickCopyActions = await firstValueFrom( this.copyButtonsService.showQuickCopyActions$, ); - const width = await firstValueFrom(this.popupWidthService.width$); + const width = await firstValueFrom(this.popupSizeService.width$); // Set initial values for the form this.appearanceForm.setValue({ @@ -187,6 +187,6 @@ export class AppearanceV2Component implements OnInit { } async updateWidth(width: PopupWidthOption) { - await this.popupWidthService.setWidth(width); + await this.popupSizeService.setWidth(width); } }