Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Desktop Modal Experiment #11484

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules โ€ฆ
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ apps/browser/src/autofill @bitwarden/team-autofill-dev
apps/desktop/src/autofill @bitwarden/team-autofill-dev
libs/common/src/autofill @bitwarden/team-autofill-dev
apps/desktop/macos/autofill-extension @bitwarden/team-autofill-dev
apps/desktop/src/app/components/fido2placeholder.component.ts @bitwarden/team-autofill-dev
# DuckDuckGo integration
apps/desktop/native-messaging-test-runner @bitwarden/team-autofill-dev
apps/desktop/src/services/duckduckgo-message-handler.service.ts @bitwarden/team-autofill-dev
Expand Down
5 changes: 5 additions & 0 deletions apps/desktop/src/app/app-routing.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import { UpdateTempPasswordComponent } from "../auth/update-temp-password.component";
import { VaultComponent } from "../vault/app/vault/vault.component";

import { Fido2PlaceholderComponent } from "./components/fido2placeholder.component";

Check warning on line 67 in apps/desktop/src/app/app-routing.module.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/app-routing.module.ts#L67

Added line #L67 was not covered by tests
import { SendComponent } from "./tools/send/send.component";

/**
Expand Down Expand Up @@ -331,6 +332,10 @@
children: [{ path: "", component: LoginDecryptionOptionsComponent }],
},
),
{
path: "passkeys",
component: Fido2PlaceholderComponent,
},
{
path: "",
component: AnonLayoutWrapperComponent,
Expand Down
28 changes: 28 additions & 0 deletions apps/desktop/src/app/components/fido2placeholder.component.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { Component } from "@angular/core";
import { Router } from "@angular/router";

Check warning on line 2 in apps/desktop/src/app/components/fido2placeholder.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/components/fido2placeholder.component.ts#L1-L2

Added lines #L1 - L2 were not covered by tests

import { DesktopSettingsService } from "../../platform/services/desktop-settings.service";

Check warning on line 4 in apps/desktop/src/app/components/fido2placeholder.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/components/fido2placeholder.component.ts#L4

Added line #L4 was not covered by tests

@Component({
standalone: true,
template: `
<div
style="background:white; display:flex; justify-content: center; align-items: center; flex-direction: column"
>
<h1>Select your passkey</h1>
<br />
<button bitButton type="button" buttonType="secondary" (click)="closeModal()">Close</button>
</div>
`,
})
export class Fido2PlaceholderComponent {

Check warning on line 18 in apps/desktop/src/app/components/fido2placeholder.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/components/fido2placeholder.component.ts#L18

Added line #L18 was not covered by tests
constructor(
private readonly desktopSettingsService: DesktopSettingsService,
private readonly router: Router,

Check warning on line 21 in apps/desktop/src/app/components/fido2placeholder.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/components/fido2placeholder.component.ts#L21

Added line #L21 was not covered by tests
) {}

async closeModal() {
await this.router.navigate(["/"]);
await this.desktopSettingsService.setInModalMode(false);

Check warning on line 26 in apps/desktop/src/app/components/fido2placeholder.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/components/fido2placeholder.component.ts#L25-L26

Added lines #L25 - L26 were not covered by tests
}
}
3 changes: 3 additions & 0 deletions apps/desktop/src/app/services/init.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import { KeyService as KeyServiceAbstraction } from "@bitwarden/key-management";

import { DesktopAutofillService } from "../../autofill/services/desktop-autofill.service";
import { DesktopSettingsService } from "../../platform/services/desktop-settings.service";

Check warning on line 24 in apps/desktop/src/app/services/init.service.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/services/init.service.ts#L24

Added line #L24 was not covered by tests
import { I18nRendererService } from "../../platform/services/i18n.renderer.service";
import { SshAgentService } from "../../platform/services/ssh-agent.service";
import { VersionService } from "../../platform/services/version.service";
Expand All @@ -47,6 +48,7 @@
private versionService: VersionService,
private sshAgentService: SshAgentService,
private autofillService: DesktopAutofillService,
private desktopSettingsService: DesktopSettingsService,
@Inject(DOCUMENT) private document: Document,
) {}

Expand Down Expand Up @@ -79,6 +81,7 @@
const htmlEl = this.win.document.documentElement;
htmlEl.classList.add("os_" + this.platformUtilsService.getDeviceString());
this.themingService.applyThemeChangesTo(this.document);
await this.desktopSettingsService.resetInModalMode();

Check warning on line 84 in apps/desktop/src/app/services/init.service.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/services/init.service.ts#L84

Added line #L84 was not covered by tests

this.versionService.init();

Expand Down
43 changes: 42 additions & 1 deletion apps/desktop/src/main/tray.main.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import * as path from "path";
import * as url from "url";

Check warning on line 4 in apps/desktop/src/main/tray.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/tray.main.ts#L4

Added line #L4 was not covered by tests

import { app, BrowserWindow, Menu, MenuItemConstructorOptions, nativeImage, Tray } from "electron";
import { firstValueFrom } from "rxjs";

import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";

import { DesktopSettingsService } from "../platform/services/desktop-settings.service";
import { cleanUserAgent, isDev } from "../utils";

Check warning on line 12 in apps/desktop/src/main/tray.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/tray.main.ts#L12

Added line #L12 was not covered by tests

import { WindowMain } from "./window.main";

Expand Down Expand Up @@ -46,6 +48,11 @@
label: this.i18nService.t("showHide"),
click: () => this.toggleWindow(),
},
{
visible: isDev(),
label: "Fake Popup",
click: () => this.fakePopup(),

Check warning on line 54 in apps/desktop/src/main/tray.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/tray.main.ts#L54

Added line #L54 was not covered by tests
},
{ type: "separator" },
{
label: this.i18nService.t("exit"),
Expand Down Expand Up @@ -183,7 +190,7 @@
this.hideDock();
}
} else {
this.windowMain.win.show();
this.windowMain.show();

Check warning on line 193 in apps/desktop/src/main/tray.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/tray.main.ts#L193

Added line #L193 was not covered by tests
if (this.isDarwin()) {
this.showDock();
}
Expand All @@ -196,4 +203,38 @@
this.windowMain.win.close();
}
}

/**
* This method is used to test modal behavior during development and could be removed in the future.
* @returns
*/
private async fakePopup() {
if (this.windowMain.win == null || this.windowMain.win.isDestroyed()) {
await this.windowMain.createWindow("modal-app");
return;

Check warning on line 214 in apps/desktop/src/main/tray.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/tray.main.ts#L213-L214

Added lines #L213 - L214 were not covered by tests
}

// Restyle existing
const existingWin = this.windowMain.win;

Check warning on line 218 in apps/desktop/src/main/tray.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/tray.main.ts#L218

Added line #L218 was not covered by tests

await this.desktopSettingsService.setInModalMode(true);
await existingWin.loadURL(

Check warning on line 221 in apps/desktop/src/main/tray.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/tray.main.ts#L220-L221

Added lines #L220 - L221 were not covered by tests
url.format({
protocol: "file:",
//pathname: `${__dirname}/index.html`,
pathname: path.join(__dirname, "/index.html"),
slashes: true,
hash: "/passkeys",
query: {
redirectUrl: "/passkeys",
},
}),
{
userAgent: cleanUserAgent(existingWin.webContents.userAgent),
},
);
existingWin.once("ready-to-show", () => {
existingWin.show();

Check warning on line 237 in apps/desktop/src/main/tray.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/tray.main.ts#L236-L237

Added lines #L236 - L237 were not covered by tests
});
}
}
100 changes: 82 additions & 18 deletions apps/desktop/src/main/window.main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@
import * as url from "url";

import { app, BrowserWindow, ipcMain, nativeTheme, screen, session } from "electron";
import { firstValueFrom } from "rxjs";
import { concatMap, firstValueFrom, pairwise } from "rxjs";

Check warning on line 8 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L8

Added line #L8 was not covered by tests

import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service";
import { processisolations } from "@bitwarden/desktop-napi";
import { BiometricStateService } from "@bitwarden/key-management";

import { WindowState } from "../platform/models/domain/window-state";
import { applyMainWindowStyles, applyPopupModalStyles } from "../platform/popup-modal-styles";

Check warning on line 16 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L16

Added line #L16 was not covered by tests
import { DesktopSettingsService } from "../platform/services/desktop-settings.service";
import { cleanUserAgent, isDev, isLinux, isMac, isMacAppStore, isWindows } from "../utils";

Expand Down Expand Up @@ -76,6 +77,24 @@
}
});

this.desktopSettingsService.inModalMode$

Check warning on line 80 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L80

Added line #L80 was not covered by tests
.pipe(
pairwise(),
concatMap(async ([lastValue, newValue]) => {

Check warning on line 83 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L83

Added line #L83 was not covered by tests
if (lastValue && !newValue) {
// Reset the window state to the main window state
applyMainWindowStyles(this.win, this.windowStates[mainWindowSizeKey]);

Check warning on line 86 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L86

Added line #L86 was not covered by tests
// Because modal is used in front of another app, UX wise it makes sense to hide the main window when leaving modal mode.
this.win.hide();

Check warning on line 88 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L88

Added line #L88 was not covered by tests
abergs marked this conversation as resolved.
Show resolved Hide resolved
} else if (!lastValue && newValue) {
// Apply the popup modal styles
applyPopupModalStyles(this.win);
this.win.show();

Check warning on line 92 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L91-L92

Added lines #L91 - L92 were not covered by tests
}
}),
)
.subscribe();

return new Promise<void>((resolve, reject) => {
try {
if (!isMacAppStore()) {
Expand Down Expand Up @@ -175,7 +194,15 @@
});
}

async createWindow(): Promise<void> {
/// Show the window with main window styles
show() {
if (this.win != null) {
applyMainWindowStyles(this.win, this.windowStates[mainWindowSizeKey]);
this.win.show();

Check warning on line 201 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L200-L201

Added lines #L200 - L201 were not covered by tests
}
}

async createWindow(template: "full-app" | "modal-app" = "full-app"): Promise<void> {
this.windowStates[mainWindowSizeKey] = await this.getWindowState(
this.defaultWidth,
this.defaultHeight,
Expand Down Expand Up @@ -209,6 +236,12 @@
},
});

if (template === "modal-app") {
applyPopupModalStyles(this.win);

Check warning on line 240 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L240

Added line #L240 was not covered by tests
} else {
applyMainWindowStyles(this.win, this.windowStates[mainWindowSizeKey]);

Check warning on line 242 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L242

Added line #L242 was not covered by tests
}

this.win.webContents.on("dom-ready", () => {
this.win.webContents.zoomFactor = this.windowStates[mainWindowSizeKey].zoomFactor ?? 1.0;
});
Expand All @@ -218,21 +251,41 @@
}

// Show it later since it might need to be maximized.
this.win.show();

// and load the index.html of the app.
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.win.loadURL(
url.format({
protocol: "file:",
pathname: path.join(__dirname, "/index.html"),
slashes: true,
}),
{
userAgent: cleanUserAgent(this.win.webContents.userAgent),
},
);
// use once event to avoid flash on unstyled content.
this.win.once("ready-to-show", () => {
this.win.show();

Check warning on line 256 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L255-L256

Added lines #L255 - L256 were not covered by tests
});

if (template === "full-app") {
// and load the index.html of the app.
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
void this.win.loadURL(

Check warning on line 262 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L262

Added line #L262 was not covered by tests
url.format({
protocol: "file:",
pathname: path.join(__dirname, "/index.html"),
slashes: true,
}),
{
userAgent: cleanUserAgent(this.win.webContents.userAgent),
},
);
} else {
// we're in modal mode - load the passkeys page
await this.win.loadURL(

Check warning on line 274 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L274

Added line #L274 was not covered by tests
url.format({
protocol: "file:",
pathname: path.join(__dirname, "/index.html"),
slashes: true,
hash: "/passkeys",
query: {
redirectUrl: "/passkeys",
},
}),
{
userAgent: cleanUserAgent(this.win.webContents.userAgent),
},
);
}

// Open the DevTools.
if (isDev()) {
Expand Down Expand Up @@ -319,6 +372,12 @@
return;
}

const inModalMode = await firstValueFrom(this.desktopSettingsService.inModalMode$);

Check warning on line 375 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L375

Added line #L375 was not covered by tests

if (inModalMode) {
return;

Check warning on line 378 in apps/desktop/src/main/window.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/window.main.ts#L378

Added line #L378 was not covered by tests
}

try {
const bounds = win.getBounds();

Expand All @@ -329,9 +388,14 @@
}
}

this.windowStates[configKey].isMaximized = win.isMaximized();
// We treat fullscreen as maximized (would be even better to store isFullscreen as its own flag).
this.windowStates[configKey].isMaximized = win.isMaximized() || win.isFullScreen();
this.windowStates[configKey].displayBounds = screen.getDisplayMatching(bounds).bounds;

// Maybe store these as well?
// win.isFocused();
// win.isVisible();

if (!win.isMaximized() && !win.isMinimized() && !win.isFullScreen()) {
this.windowStates[configKey].x = bounds.x;
this.windowStates[configKey].y = bounds.y;
Expand Down
52 changes: 52 additions & 0 deletions apps/desktop/src/platform/popup-modal-styles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { BrowserWindow } from "electron";

import { WindowState } from "./models/domain/window-state";

// change as needed, however limited by mainwindow minimum size
const popupWidth = 680;
const popupHeight = 500;

Check warning on line 7 in apps/desktop/src/platform/popup-modal-styles.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/platform/popup-modal-styles.ts#L6-L7

Added lines #L6 - L7 were not covered by tests
coroiu marked this conversation as resolved.
Show resolved Hide resolved

export function applyPopupModalStyles(window: BrowserWindow) {
window.unmaximize();
window.setSize(popupWidth, popupHeight);
window.center();

Check warning on line 12 in apps/desktop/src/platform/popup-modal-styles.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/platform/popup-modal-styles.ts#L9-L12

Added lines #L9 - L12 were not covered by tests
window.setWindowButtonVisibility?.(false);
window.setMenuBarVisibility?.(false);
window.setResizable(false);
window.setAlwaysOnTop(true);

Check warning on line 16 in apps/desktop/src/platform/popup-modal-styles.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/platform/popup-modal-styles.ts#L15-L16

Added lines #L15 - L16 were not covered by tests

// Adjusting from full screen is a bit more hassle
if (window.isFullScreen()) {
window.setFullScreen(false);
window.once("leave-full-screen", () => {
window.setSize(popupWidth, popupHeight);
window.center();

Check warning on line 23 in apps/desktop/src/platform/popup-modal-styles.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/platform/popup-modal-styles.ts#L20-L23

Added lines #L20 - L23 were not covered by tests
});
}
}

export function applyMainWindowStyles(window: BrowserWindow, existingWindowState: WindowState) {
window.setMinimumSize(680, 500);

Check warning on line 29 in apps/desktop/src/platform/popup-modal-styles.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/platform/popup-modal-styles.ts#L28-L29

Added lines #L28 - L29 were not covered by tests

// need to guard against null/undefined values

if (existingWindowState?.width && existingWindowState?.height) {
window.setSize(existingWindowState.width, existingWindowState.height);

Check warning on line 34 in apps/desktop/src/platform/popup-modal-styles.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/platform/popup-modal-styles.ts#L34

Added line #L34 was not covered by tests
}

if (existingWindowState?.x && existingWindowState?.y) {
window.setPosition(existingWindowState.x, existingWindowState.y);

Check warning on line 38 in apps/desktop/src/platform/popup-modal-styles.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/platform/popup-modal-styles.ts#L38

Added line #L38 was not covered by tests
}

window.setWindowButtonVisibility?.(true);
window.setMenuBarVisibility?.(true);
window.setResizable(true);
window.setAlwaysOnTop(false);

Check warning on line 44 in apps/desktop/src/platform/popup-modal-styles.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/platform/popup-modal-styles.ts#L43-L44

Added lines #L43 - L44 were not covered by tests

// We're currently not recovering the maximized state, mostly due to conflicts with hiding the window.
// window.setFullScreen(existingWindowState.isMaximized);

// if (existingWindowState.isMaximized) {
// window.maximize();
// }
}
Loading
Loading