#8293 Unavailable overlay for sidebar forms and temp panels on nav (#…

…8351)

* Unavailable overlay for sidebar forms and temp panels on nav

* close button works, added tests

* fixing lint errors

* wip

* PR comments

.eslintrc.js

@@ -154,6 +154,14 @@ module.exports = {
             allowConditional: true,
           },
         ],
+        "playwright/no-wait-for-timeout": "error",
+        "playwright/no-useless-not": "error",
+        "playwright/expect-expect": [
+          "error",
+          { assertFunctionNames: ["checkUnavailibilityForNavigationMethod"] },
+        ],
+        "playwright/no-conditional-in-test": "error",
+        "playwright/no-conditional-expect": "error",
         "playwright/no-commented-out-tests": "error",
         "playwright/no-hooks": "error", // Use fixtures instead to share common setup / teardown code
         "playwright/no-get-by-title": "error",

end-to-end-tests/fixtures/envSetup.ts

@@ -28,6 +28,7 @@ export const test = base.extend<{
       assertRequiredEnvVariables();
 
       for (const key of additionalRequiredEnvVariables) {
+        // eslint-disable-next-line security/detect-object-injection -- internally controlled
         if (process.env[key] === undefined) {
           throw new Error(
             `This test requires additional environment variable ${key} to be configured. Configure it in your .env.development file and re-build the extension.`,

end-to-end-tests/tests/bricks/sidebarEffects.spec.ts

@@ -32,7 +32,6 @@ test.describe("sidebar effect bricks", () => {
     await page.goto("/");
 
     // Ensure the page is focused by clicking on an element before running the keyboard shortcut, see runModViaQuickbar
-    await page.getByText("Index of  /").click();
     await runModViaQuickBar(page, "Toggle Sidebar");
 
     // Will error if page/frame not available

end-to-end-tests/tests/extensionConsoleActivation.spec.ts

@@ -83,8 +83,11 @@ test("can activate a mod with built-in integration", async ({
   await modActivationPage.clickActivateAndWaitForModsPageRedirect();
   await page.goto("/");
 
-  // Ensure the page is focused by clicking on an element before running the keyboard shortcut, see runModViaQuickbar
-  await page.getByText("Index of  /").click();
+  // Ensure the QuickBar is ready
+  await expect(
+    page.getByRole("button", { name: "open the PixieBrix quick bar" }),
+  ).toBeVisible();
+
   await runModViaQuickBar(page, "GIPHY Search");
 
   // Search for "kitten" keyword

end-to-end-tests/tests/regressions/formFlicker.spec.ts

@@ -41,7 +41,6 @@ test.describe("forms flickering due to components unexpectedly unmounting/remoun
 
     await page.goto("/bootstrap-5");
 
-    await page.getByRole("heading", { name: "PixieBrix" }).click();
     await runModViaQuickBar(page, "Open Sidebar");
 
     const sideBarPage = await getSidebarPage(page, extensionId);

end-to-end-tests/tests/runtime/sidebarController.spec.ts

@@ -70,7 +70,7 @@ test.describe("sidebar controller", () => {
 
     // The focus dialog should not be shown in the iframe. Check after checking the top-level frame
     // because it's a positive check for the dialog being shown.
-    await expect(frame.getByRole("button", { name: "OK" })).not.toBeVisible();
+    await expect(frame.getByRole("button", { name: "OK" })).toBeHidden();
 
     // Will error if page/frame not available
     await getSidebarPage(page, extensionId);

end-to-end-tests/tests/runtime/sidebarNavigation.spec.ts

@@ -22,7 +22,7 @@ import { type Page, test as base } from "@playwright/test";
 import { getSidebarPage, runModViaQuickBar } from "../../utils";
 import { MV, SERVICE_URL } from "../../env";
 
-test("sidebar is persistent during navigation", async ({
+test("sidebar mod panels are persistent during navigation", async ({
   page,
   extensionId,
 }) => {
@@ -38,8 +38,6 @@ test("sidebar is persistent during navigation", async ({
 
   await page.goto("/");
 
-  // Ensure the page is focused by clicking on an element before running the keyboard shortcut, see runModViaQuickbar
-  await page.getByText("Index of  /").click();
   await runModViaQuickBar(page, "Open Sidebar");
 
   const sideBarPage = (await getSidebarPage(page, extensionId)) as Page; // MV3 sidebar is a separate page
@@ -108,3 +106,133 @@ test("sidebar is persistent during navigation", async ({
     expect(sideBarPageClosed).toBe(true);
   }).toPass({ timeout: 5000 });
 });
+
+const navigationMethods: Array<{
+  name: string;
+  navigationMethod: (page: Page) => Promise<void>;
+}> = [
+  {
+    name: "refresh",
+    async navigationMethod(page: Page) {
+      await page.reload();
+    },
+  },
+  {
+    name: "back button",
+    async navigationMethod(page: Page) {
+      await page.goBack();
+      await page.goBack();
+    },
+  },
+  {
+    name: "goto new page",
+    async navigationMethod(page: Page) {
+      await page.goto(SERVICE_URL);
+    },
+  },
+];
+
+// Helper method for checking that the sidebar panels are unavailable after a navigation method
+async function checkUnavailibilityForNavigationMethod(
+  page: Page,
+  extensionId: string,
+  navigationMethod: (page: Page) => Promise<void>,
+) {
+  await page.goto("/advanced-fields");
+  await runModViaQuickBar(page, "Open form");
+
+  const sideBarPage = (await getSidebarPage(page, extensionId)) as Page; // MV3 sidebar is a separate page
+  // Set up close listener for sidebar page
+  let sideBarPageClosed = false;
+  sideBarPage.on("close", () => {
+    sideBarPageClosed = true;
+  });
+
+  await expect(
+    sideBarPage
+      .frameLocator("iframe")
+      .getByRole("heading", { name: "Example Form" }),
+  ).toBeVisible();
+  await expect(
+    sideBarPage.getByRole("tab", { name: "Example form" }),
+  ).toBeVisible();
+
+  await runModViaQuickBar(page, "Open temp panel");
+  await expect(
+    sideBarPage.getByRole("heading", { name: "Example document" }),
+  ).toBeVisible();
+  await expect(
+    sideBarPage.getByRole("tab", { name: "Example info" }),
+  ).toBeVisible();
+
+  // Click on "contentEditable" header, which updates the url to .../#contenteditable
+  await page.getByRole("link", { name: "contentEditable" }).click();
+  expect(page.url()).toBe(
+    "https://pbx.vercel.app/advanced-fields/#contenteditable",
+  );
+  // Should not cause the temporary panel to become unavailable
+  await expect(
+    sideBarPage
+      .getByLabel("Example Info")
+      .getByText("Panel no longer available"),
+  ).toBeHidden();
+
+  await navigationMethod(page);
+
+  await expect(
+    sideBarPage
+      .getByLabel("Example Info")
+      .getByText("Panel no longer available"),
+  ).toBeVisible();
+  await sideBarPage
+    .getByLabel("Example Info")
+    .getByLabel("Close the unavailable panel")
+    .click();
+  await expect(
+    sideBarPage.getByRole("tab", { name: "Example info" }),
+  ).toBeHidden();
+
+  // The unavailable overlay is still displayed for the form panel
+  await expect(
+    sideBarPage
+      .getByLabel("Example form")
+      .getByText("Panel no longer available"),
+  ).toBeVisible();
+  await sideBarPage
+    .getByLabel("Example form")
+    .getByLabel("Close the unavailable panel")
+    .click();
+
+  // Closing the last panel should close the sidebar
+  await expect(() => {
+    expect(sideBarPageClosed).toBe(true);
+  }).toPass({ timeout: 5000 });
+}
+
+test("sidebar form and temporary panels are unavailable after navigation", async ({
+  page,
+  extensionId,
+}) => {
+  test.skip(MV === "2", "Navigation is not supported for MV2 sidebar");
+  // This mod has two quickbar actions for opening a temporary panel and a form panel in the sidebar.
+  const modId = "@e2e-testing/temp-panel-unavailable-on-navigation";
+
+  const modActivationPage = new ActivateModPage(page, extensionId, modId);
+  await modActivationPage.goto();
+
+  await modActivationPage.clickActivateAndWaitForModsPageRedirect();
+
+  // Prime the browser history with an initial navigation
+  await page.goto(SERVICE_URL);
+
+  for (const { navigationMethod, name } of navigationMethods) {
+    // eslint-disable-next-line no-await-in-loop -- check each navigation method sequentially
+    await test.step(`Checking navigation method: ${name}`, async () => {
+      await checkUnavailibilityForNavigationMethod(
+        page,
+        extensionId,
+        navigationMethod,
+      );
+    });
+  }
+});

end-to-end-tests/tests/telemetry/errors.spec.ts

@@ -1,10 +1,37 @@
 import { test, expect } from "../../fixtures/extensionBase";
-import { type Request } from "playwright-core";
 // @ts-expect-error -- https://youtrack.jetbrains.com/issue/AQUA-711/Provide-a-run-configuration-for-Playwright-tests-in-specs-with-fixture-imports-only
-import { type Page, test as base } from "@playwright/test";
+import { type BrowserContext, type Page, test as base } from "@playwright/test";
 import { getBaseExtensionConsoleUrl } from "../../pageObjects/constants";
 import { MV } from "../../env";
 
+async function waitForBackgroundPageRequest(
+  context: BrowserContext,
+  extensionId: string,
+  errorServiceEndpoint: string,
+) {
+  if (MV === "3") {
+    // Due to service worker limitations with the Datadog SDK, we need to report errors via an offscreen document
+    // (see https://github.com/pixiebrix/pixiebrix-extension/issues/8268). The offscreen document is created when
+    // the first error is reported, so we need to wait for it to be created before we can interact with it.
+    let offscreenPage: Page | undefined;
+    await expect(async () => {
+      offscreenPage = context
+        .pages()
+        .find((value) =>
+          value
+            .url()
+            .startsWith(`chrome-extension://${extensionId}/offscreen.html`),
+        );
+
+      expect(offscreenPage?.url()).toBeDefined();
+    }).toPass({ timeout: 5000 });
+    return offscreenPage?.waitForRequest(errorServiceEndpoint);
+  }
+
+  const backgroundPage = context.backgroundPages()[0];
+  return backgroundPage?.waitForRequest(errorServiceEndpoint);
+}
+
 test.use({
   additionalRequiredEnvVariables: [
     "DATADOG_CLIENT_TOKEN",
@@ -39,32 +66,14 @@ test("can report application error to telemetry service", async ({
   await page.goto(getBaseExtensionConsoleUrl(extensionId));
   await expect(page.getByText("Something went wrong.")).toBeVisible();
 
-  let waitForRequest: Promise<Request> | undefined;
-  if (MV === "3") {
-    // Due to service worker limitations with the Datadog SDK, we need to report errors via an offscreen document
-    // (see https://github.com/pixiebrix/pixiebrix-extension/issues/8268). The offscreen document is created when
-    // the first error is reported, so we need to wait for it to be created before we can interact with it.
-    let offscreenPage: Page | undefined;
-    await expect(async () => {
-      offscreenPage = context
-        .pages()
-        .find((value) =>
-          value
-            .url()
-            .startsWith(`chrome-extension://${extensionId}/offscreen.html`),
-        );
-
-      expect(offscreenPage?.url()).toBeDefined();
-    }).toPass({ timeout: 5000 });
-    waitForRequest = offscreenPage?.waitForRequest(errorServiceEndpoint);
-  } else {
-    const backgroundPage = context.backgroundPages()[0];
-    waitForRequest = backgroundPage?.waitForRequest(errorServiceEndpoint);
-  }
-
   // TODO: due to Datadog SDK implementation, it will take ~30 seconds for the
   //  request to be sent. We should figure out a way to induce the request to be sent sooner.
-  const request = await waitForRequest;
+  const request = await waitForBackgroundPageRequest(
+    context,
+    extensionId,
+    errorServiceEndpoint,
+  );
+
   const errorLogsJson = request
     ?.postData()
     ?.split("\n")

end-to-end-tests/utils.ts

@@ -68,11 +68,14 @@ export async function ensureVisibility(
 }
 
 // Run a mod via the Quickbar.
-// NOTE: Page needs to be focused before running this function, e.g. by clicking on the page.
-// TODO: Fix the page-focus precondition by generalizing the page-focusing logic to be page-agnostic
 export async function runModViaQuickBar(page: Page, modName: string) {
+  await waitForQuickBarReadiness(page);
+  await page.locator("html").focus(); // Ensure the page is focused before running the keyboard shortcut
   await page.keyboard.press("Meta+M"); // MacOS
   await page.keyboard.press("Control+M"); // Windows and Linux
+  // Short delay to allow the quickbar to finish opening
+  // eslint-disable-next-line playwright/no-wait-for-timeout -- TODO: Find a better way to detect when the quickbar is done loading opening
+  await page.waitForTimeout(500);
   await page.getByRole("option", { name: modName }).click();
 }
 
@@ -161,6 +164,15 @@ export async function waitForSelectionMenuReadiness(page: Page) {
   }).toPass({ timeout: 5000 });
 }
 
+// Waits for the quick bar to be ready to use
+async function waitForQuickBarReadiness(page: Page) {
+  await expect(async () => {
+    await expect(page.locator("html")).toHaveAttribute(
+      "data-pb-quick-bar-ready",
+    );
+  }).toPass({ timeout: 5000 });
+}
+
 // Simulates mouse entering the sidebar to track focus on MV2
 // https://github.com/pixiebrix/pixiebrix-extension/blob/1794863937f343fbc8e3a4434eace74191f8dfbd/src/contentScript/sidebarController.tsx#L563-L563
 export async function conditionallyHoverOverMV2Sidebar(page: Page) {

src/components/quickBar/QuickBarApp.tsx

@@ -33,6 +33,7 @@ import { once } from "lodash";
 import {
   MAX_Z_INDEX,
   PIXIEBRIX_QUICK_BAR_CONTAINER_CLASS,
+  QUICK_BAR_READY_ATTRIBUTE,
 } from "@/domConstants";
 import useEventListener from "@/hooks/useEventListener";
 import { Stylesheets } from "@/components/Stylesheets";
@@ -203,6 +204,11 @@ export const QuickBarApp: React.FC = () => (
   </KBarProvider>
 );
 
+function markQuickBarReady() {
+  const html = globalThis.document?.documentElement;
+  html.setAttribute(QUICK_BAR_READY_ATTRIBUTE, "true");
+}
+
 export const initQuickBarApp = once(async () => {
   expectContext("contentScript");
 
@@ -226,6 +232,8 @@ export const initQuickBarApp = once(async () => {
   ReactDOM.render(<QuickBarApp />, container);
   console.debug("Initialized quick bar");
 
+  markQuickBarReady();
+
   onContextInvalidated.addListener(() => {
     console.debug("Removed quick bar due to context invalidation");
     ReactDOM.unmountComponentAtNode(container);

src/domConstants.ts

@@ -23,6 +23,8 @@ export const MAX_Z_INDEX = NOTIFICATIONS_Z_INDEX - 1; // Let notifications alway
 
 export const SELECTION_MENU_READY_ATTRIBUTE = "data-pb-selection-menu-ready";
 
+export const QUICK_BAR_READY_ATTRIBUTE = "data-pb-quick-bar-ready";
+
 export const PANEL_FRAME_ID = "pixiebrix-extension";
 
 export const PIXIEBRIX_DATA_ATTR = "data-pb-uuid";