-
Notifications
You must be signed in to change notification settings - Fork 23
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
#8293 Unavailable overlay for sidebar forms and temp panels on nav #8351
Changes from 5 commits
b109943
ba987c4
e4dc7e3
0c94e51
0832c12
ab01cd6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated lint error fixed |
||
|
||
// Will error if page/frame not available | ||
await getSidebarPage(page, extensionId); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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, | ||
) { | ||
Comment on lines
+7
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. refactored this method in the process of fixing an eslint error for no conditionals |
||
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") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without this check, sometimes the quickbar would not open when immediately attempting to open it after loading a new 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) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored the runModViaQuickBar method so we don't have to have these
click
calls before it.