From fb860cd975d42fcd2c255f62d248877bcf5d3abc Mon Sep 17 00:00:00 2001
From: John Harrington <84741727+harr1424@users.noreply.github.com>
Date: Tue, 16 Dec 2025 09:05:14 -0700
Subject: [PATCH] address review comments

---
 .../src/chromium/platform/sandbox.rs          |  5 +--
 .../tools/import/chromium-importer.service.ts |  6 +--
 .../import/import-desktop.component.html      | 24 ++++--------
 .../tools/import/import-desktop.component.ts  |  9 +----
 .../chrome/import-chrome.component.ts         | 39 +++++++------------
 5 files changed, 24 insertions(+), 59 deletions(-)

diff --git a/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/sandbox.rs b/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/sandbox.rs
index e9da982b7b7..fec37920b88 100644
--- a/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/sandbox.rs
+++ b/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/sandbox.rs
@@ -22,7 +22,6 @@ struct CheckBrowserInstalledResponse {
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase", tag = "type")]
 enum CommandResult<T> {
-    // rename = "camelCase" was a review suggestion with breaking changes
     Success { value: T },
     Error { error: String },
 }
@@ -180,9 +179,7 @@ async fn is_browser_installed(browser_name: &str) -> Result<bool> {
         .map(|(_, id)| *id);
 
     let Some(bundle_id) = bundle_id else {
-        return Ok(true); // ok_or(true) was a review suggestion with breaking changes
-                         // Avoid ok_or(true): serializes Result as {"Ok": "..."} instead of string
-                         // value
+        return Ok(true);
     };
 
     let input = CommandInput {
diff --git a/apps/desktop/src/app/tools/import/chromium-importer.service.ts b/apps/desktop/src/app/tools/import/chromium-importer.service.ts
index cdf379d5952..fd111aa7c3a 100644
--- a/apps/desktop/src/app/tools/import/chromium-importer.service.ts
+++ b/apps/desktop/src/app/tools/import/chromium-importer.service.ts
@@ -12,11 +12,7 @@ export class ChromiumImporterService {
 
     // Used on Mac OS App Store builds to request permissions to browser entries outside the sandbox
     ipcMain.handle("chromium_importer.requestBrowserAccess", async (event, browser: string) => {
-      if (chromium_importer.requestBrowserAccess) {
-        return await chromium_importer.requestBrowserAccess(browser, isMacAppStore());
-      }
-      // requestBrowserAccess not found, returning with no-op
-      return;
+      return await chromium_importer.requestBrowserAccess(browser, isMacAppStore());
     });
 
     ipcMain.handle("chromium_importer.getAvailableProfiles", async (event, browser: string) => {
diff --git a/apps/desktop/src/app/tools/import/import-desktop.component.html b/apps/desktop/src/app/tools/import/import-desktop.component.html
index 13fd29c900f..70f64c74574 100644
--- a/apps/desktop/src/app/tools/import/import-desktop.component.html
+++ b/apps/desktop/src/app/tools/import/import-desktop.component.html
@@ -1,21 +1,13 @@
-<bit-dialog #dialog dialogSize="large" background="alt">
+<bit-dialog #dialog [loading]="loading" dialogSize="large" background="alt">
   <span bitDialogTitle>{{ "import" | i18n }}</span>
   <ng-container bitDialogContent>
-    <div class="tw-relative">
-      <tools-import
-        (formLoading)="this.loading = $event"
-        (formDisabled)="this.disabled = $event"
-        (onSuccessfulImport)="this.onSuccessfulImport($event)"
-        [onImportFromBrowser]="this.onImportFromBrowser"
-        [onLoadProfilesFromBrowser]="this.onLoadProfilesFromBrowser"
-        [class.tw-invisible]="loading"
-      ></tools-import>
-      @if (loading) {
-        <div class="tw-absolute tw-inset-0 tw-flex tw-items-center tw-justify-center">
-          <bit-spinner></bit-spinner>
-        </div>
-      }
-    </div>
+    <tools-import
+      (formLoading)="this.loading = $event"
+      (formDisabled)="this.disabled = $event"
+      (onSuccessfulImport)="this.onSuccessfulImport($event)"
+      [onImportFromBrowser]="this.onImportFromBrowser"
+      [onLoadProfilesFromBrowser]="this.onLoadProfilesFromBrowser"
+    ></tools-import>
   </ng-container>
   <ng-container bitDialogFooter>
     <button
diff --git a/apps/desktop/src/app/tools/import/import-desktop.component.ts b/apps/desktop/src/app/tools/import/import-desktop.component.ts
index 0e1022d6c78..5b0fc4707aa 100644
--- a/apps/desktop/src/app/tools/import/import-desktop.component.ts
+++ b/apps/desktop/src/app/tools/import/import-desktop.component.ts
@@ -3,13 +3,7 @@ import { Component } from "@angular/core";
 
 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
-import {
-  DialogRef,
-  AsyncActionsModule,
-  ButtonModule,
-  DialogModule,
-  SpinnerComponent,
-} from "@bitwarden/components";
+import { DialogRef, AsyncActionsModule, ButtonModule, DialogModule } from "@bitwarden/components";
 import type { chromium_importer } from "@bitwarden/desktop-napi";
 import { ImportMetadataServiceAbstraction } from "@bitwarden/importer-core";
 import {
@@ -32,7 +26,6 @@ import { DesktopImportMetadataService } from "./desktop-import-metadata.service"
     AsyncActionsModule,
     ButtonModule,
     ImportComponent,
-    SpinnerComponent,
   ],
   providers: [
     ...ImporterProviders,
diff --git a/libs/importer/src/components/chrome/import-chrome.component.ts b/libs/importer/src/components/chrome/import-chrome.component.ts
index 27a2c0bc96c..db72ea8a275 100644
--- a/libs/importer/src/components/chrome/import-chrome.component.ts
+++ b/libs/importer/src/components/chrome/import-chrome.component.ts
@@ -1,16 +1,7 @@
 // FIXME: Update this file to be type safe and remove this and next line
 // @ts-strict-ignore
 import { CommonModule } from "@angular/common";
-import {
-  Component,
-  effect,
-  EventEmitter,
-  input,
-  Input,
-  OnDestroy,
-  OnInit,
-  Output,
-} from "@angular/core";
+import { Component, EventEmitter, input, Input, OnDestroy, OnInit, Output } from "@angular/core";
 import {
   AsyncValidatorFn,
   ControlContainer,
@@ -114,26 +105,22 @@ export class ImportChromeComponent implements OnInit, OnDestroy {
     private controlContainer: ControlContainer,
     private logService: LogService,
     private i18nService: I18nService,
-  ) {
-    effect(async () => {
-      // Callback is set via @Input after constructor, so check it exists
-      if (this.onLoadProfilesFromBrowser) {
-        try {
-          this.profileList = await this.onLoadProfilesFromBrowser(
-            this.getBrowserName(this.format()),
-          );
-        } catch (error) {
-          this.logService.error("Error loading profiles from browser:", error);
-          const translatedMessage = this.translateValidationError(error);
-          this.error.emit(translatedMessage);
-        }
-      }
-    });
-  }
+  ) {}
 
   async ngOnInit(): Promise<void> {
     this._parentFormGroup = this.controlContainer.control as FormGroup;
     this._parentFormGroup.addControl("chromeOptions", this.formGroup);
+
+    // Load profiles from browser on initialization
+    if (this.onLoadProfilesFromBrowser) {
+      try {
+        this.profileList = await this.onLoadProfilesFromBrowser(this.getBrowserName(this.format()));
+      } catch (error) {
+        this.logService.error("Error loading profiles from browser:", error);
+        const translatedMessage = this.translateValidationError(error);
+        this.error.emit(translatedMessage);
+      }
+    }
   }
 
   ngOnDestroy(): void {