Skip to content

Commit

Permalink
[CL-509][PM-16190] Avoid double scrollbar appearing when default zoom…
Browse files Browse the repository at this point in the history
… is >100% (#12427)
  • Loading branch information
vleague2 authored Dec 27, 2024
1 parent 6d65ce9 commit 4f060d8
Show file tree
Hide file tree
Showing 10 changed files with 84 additions and 37 deletions.
15 changes: 15 additions & 0 deletions apps/browser/src/platform/browser/browser-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,21 @@ export class BrowserApi {
});
}

/**
* Fetch the currently open browser tab
*/
static async getCurrentTab(): Promise<chrome.tabs.Tab> | 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<chrome.tabs.Tab[]> {
return new Promise((resolve) => {
chrome.tabs.query(options, (tabs) => {
Expand Down
18 changes: 17 additions & 1 deletion apps/browser/src/platform/popup/browser-popup-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand All @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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],
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
POPUP_STYLE_DISK,
} from "@bitwarden/common/platform/state";

import BrowserPopupUtils from "../browser-popup-utils";

/**
*
* Value represents width in pixels
Expand All @@ -25,10 +27,12 @@ const POPUP_WIDTH_KEY_DEF = new KeyDefinition<PopupWidthOption>(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);

Expand All @@ -41,23 +45,39 @@ 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`;
}

/**
* 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);
}
}
3 changes: 0 additions & 3 deletions apps/browser/src/popup/app.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions apps/browser/src/popup/main.ts
Original file line number Diff line number Diff line change
@@ -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");
Expand All @@ -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");
Expand Down
18 changes: 10 additions & 8 deletions apps/browser/src/popup/scss/base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ body {
}

body {
min-width: 380px;
height: 600px !important;
width: 380px;
height: 600px;
position: relative;
min-height: 100vh;
overflow: hidden;
Expand All @@ -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%;
}
}

Expand Down
13 changes: 5 additions & 8 deletions apps/browser/src/popup/services/init.service.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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,
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -55,7 +55,7 @@ describe("AppearanceV2Component", () => {
const setEnableCompactMode = jest.fn().mockResolvedValue(undefined);
const setShowQuickCopyActions = jest.fn().mockResolvedValue(undefined);

const mockWidthService: Partial<PopupWidthService> = {
const mockWidthService: Partial<PopupSizeService> = {
width$: new BehaviorSubject("default"),
setWidth: jest.fn().mockResolvedValue(undefined),
};
Expand Down Expand Up @@ -95,7 +95,7 @@ describe("AppearanceV2Component", () => {
} as Partial<VaultPopupCopyButtonsService>,
},
{
provide: PopupWidthService,
provide: PopupSizeService,
useValue: mockWidthService,
},
],
Expand Down
10 changes: 5 additions & 5 deletions apps/browser/src/vault/popup/settings/appearance-v2.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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({
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -187,6 +187,6 @@ export class AppearanceV2Component implements OnInit {
}

async updateWidth(width: PopupWidthOption) {
await this.popupWidthService.setWidth(width);
await this.popupSizeService.setWidth(width);
}
}

0 comments on commit 4f060d8

Please sign in to comment.