-
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
#8228: run show sidebar in top-level frame #8299
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
1965263
#8228: show sidebar in top-level frame
twschiller 1018595
#8228: add test for opening sidebar from frame
twschiller fdb5fbe
#8228: add test for sidebar with wait
twschiller f0320de
Clarify method
twschiller 5bfa8bb
Merge remote-tracking branch 'origin/main' into feature/8228-sidebar-…
twschiller ad8b11c
Split tests
twschiller 51b1816
Fix focus dialog test
twschiller File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
/* | ||
* Copyright (C) 2024 PixieBrix, Inc. | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Affero General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Affero General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
import { test, expect } from "../../fixtures/extensionBase"; | ||
import { ActivateModPage } from "../../pageObjects/extensionConsole/modsPage"; | ||
// @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 { getSidebarPage, isSidebarOpen } from "../../utils"; | ||
import { MV } from "../../env"; | ||
|
||
test.describe("sidebar controller", () => { | ||
test("can open sidebar immediately from iframe without focus dialog", async ({ | ||
page, | ||
extensionId, | ||
}) => { | ||
const modId = "@pixies/test/frame-sidebar-actions"; | ||
|
||
const modActivationPage = new ActivateModPage(page, extensionId, modId); | ||
await modActivationPage.goto(); | ||
await modActivationPage.clickActivateAndWaitForModsPageRedirect(); | ||
|
||
await page.goto("/frames-builder.html"); | ||
|
||
const frame = page.frameLocator("iframe"); | ||
await frame.getByRole("link", { name: "Show Sidebar Immediately" }).click(); | ||
|
||
// Don't use getSidebarPage because it automatically clicks the MV3 focus dialog. | ||
await expect(() => { | ||
expect(isSidebarOpen(page, extensionId)).toBe(false); | ||
}).toPass({ timeout: 5000 }); | ||
}); | ||
|
||
test("shows focus dialog in top-level frame", async ({ | ||
page, | ||
extensionId, | ||
}) => { | ||
test.skip(MV === "2", "This test is only relevant for MV3"); | ||
|
||
const modId = "@pixies/test/frame-sidebar-actions"; | ||
|
||
const modActivationPage = new ActivateModPage(page, extensionId, modId); | ||
await modActivationPage.goto(); | ||
await modActivationPage.clickActivateAndWaitForModsPageRedirect(); | ||
|
||
await page.goto("/frames-builder.html"); | ||
|
||
const frame = page.frameLocator("iframe"); | ||
|
||
// Mod waits 7.5 seconds before running Show Sidebar brick to ensure the user gesture dialog is shown | ||
await frame.getByRole("link", { name: "Show Sidebar after Wait" }).click(); | ||
// eslint-disable-next-line playwright/no-wait-for-timeout -- match wait in the mod | ||
await page.waitForTimeout(8000); | ||
|
||
// Focus dialog should be visible in the top-level frame | ||
await expect(page.getByRole("button", { name: "OK" })).toBeVisible(); | ||
|
||
// 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(); | ||
|
||
// Will error if page/frame not available | ||
await getSidebarPage(page, extensionId); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,19 +19,21 @@ import reportEvent from "@/telemetry/reportEvent"; | |
import { Events } from "@/telemetry/events"; | ||
import { expectContext } from "@/utils/expectContext"; | ||
import sidebarInThisTab from "@/sidebar/messenger/api"; | ||
import * as contentScriptApi from "@/contentScript/messenger/strict/api"; | ||
import { isEmpty, throttle } from "lodash"; | ||
import { signalFromEvent } from "abort-utils"; | ||
import { SimpleEventTarget } from "@/utils/SimpleEventTarget"; | ||
import * as sidebarMv2 from "@/contentScript/sidebarDomControllerLite"; | ||
import { getSidebarElement } from "@/contentScript/sidebarDomControllerLite"; | ||
import { type Except } from "type-fest"; | ||
import { type RunArgs, RunReason } from "@/types/runtimeTypes"; | ||
import { type UUID } from "@/types/stringTypes"; | ||
import { type RegistryId } from "@/types/registryTypes"; | ||
import { type ModComponentRef } from "@/types/modComponentTypes"; | ||
import type { | ||
ActivatePanelOptions, | ||
ModActivationPanelEntry, | ||
FormPanelEntry, | ||
ModActivationPanelEntry, | ||
PanelEntry, | ||
PanelPayload, | ||
TemporaryPanelEntry, | ||
|
@@ -41,18 +43,29 @@ import { getFormPanelSidebarEntries } from "@/platform/forms/formController"; | |
import { memoizeUntilSettled } from "@/utils/promiseUtils"; | ||
import { getTimedSequence } from "@/types/helpers"; | ||
import { isMV3 } from "@/mv3/api"; | ||
import { getErrorMessage } from "@/errors/errorHelpers"; | ||
import { focusCaptureDialog } from "@/contentScript/focusCaptureDialog"; | ||
import { isLoadedInIframe } from "@/utils/iframeUtils"; | ||
import { showMySidePanel } from "@/background/messenger/strict/api"; | ||
import { getSidebarElement } from "@/contentScript/sidebarDomControllerLite"; | ||
import focusController from "@/utils/focusController"; | ||
import selectionController from "@/utils/selectionController"; | ||
import { messenger } from "webext-messenger"; | ||
import { getSidebarTargetForCurrentTab } from "@/utils/sidePanelUtils"; | ||
import { getTopLevelFrame, messenger } from "webext-messenger"; | ||
import { | ||
getSidebarTargetForCurrentTab, | ||
isUserGestureRequiredError, | ||
} from "@/utils/sidePanelUtils"; | ||
|
||
const HIDE_SIDEBAR_EVENT_NAME = "pixiebrix:hideSidebar"; | ||
|
||
/** | ||
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. Moved module variables to top of file |
||
* Event listeners triggered when the sidebar shows and is ready to receive messages. | ||
*/ | ||
export const sidebarShowEvents = new SimpleEventTarget<RunArgs>(); | ||
|
||
// eslint-disable-next-line local-rules/persistBackgroundData -- Unused there | ||
const panels: PanelEntry[] = []; | ||
|
||
let modActivationPanelEntry: ModActivationPanelEntry | null = null; | ||
|
||
/* | ||
* Only one check at a time | ||
* Cannot throttle because subsequent checks need to be able to be made immediately | ||
|
@@ -88,30 +101,37 @@ const pingSidebar = memoizeUntilSettled( | |
}, 1000) as () => Promise<void>, | ||
); | ||
|
||
/** | ||
* Event listeners triggered when the sidebar shows and is ready to receive messages. | ||
*/ | ||
export const sidebarShowEvents = new SimpleEventTarget<RunArgs>(); | ||
|
||
export function sidebarWasLoaded(): void { | ||
sidebarShowEvents.emit({ reason: RunReason.MANUAL }); | ||
} | ||
|
||
// eslint-disable-next-line local-rules/persistBackgroundData -- Unused there | ||
const panels: PanelEntry[] = []; | ||
|
||
let modActivationPanelEntry: ModActivationPanelEntry | null = null; | ||
|
||
/** | ||
* Attach the sidebar to the page if it's not already attached. Then re-renders all panels. | ||
* Content script handler for showing the sidebar in the top-level frame. Regular callers should call | ||
* showSidebar instead, which handles calls from iframes. | ||
* | ||
* - Resolves when the sidebar is initialized (responds to a ping) | ||
* - Shows focusCaptureDialog if a user gesture is required | ||
* | ||
* @see showSidebar | ||
* @see pingSidebar | ||
* @throws Error if the sidebar ping fails or does not respond in time | ||
*/ | ||
export async function showSidebar(): Promise<void> { | ||
// Don't memoizeUntilSettled this method. focusCaptureDialog is memoized which prevents this method from showing | ||
// the focus dialog from multiple times. By allowing multiple concurrent calls to showSidebarInTopFrame, | ||
// a subsequent call might succeed, which will then automatically close the focusCaptureDialog (via it's abort signal) | ||
export async function showSidebarInTopFrame() { | ||
reportEvent(Events.SIDEBAR_SHOW); | ||
|
||
if (isLoadedInIframe()) { | ||
console.warn("showSidebarInTopFrame should not be called in an iframe"); | ||
} | ||
|
||
// Defensively handle accidental calls from iframes | ||
if (isMV3() || isLoadedInIframe()) { | ||
try { | ||
await showMySidePanel(); | ||
} catch (error) { | ||
if (!getErrorMessage(error).includes("user gesture")) { | ||
if (!isUserGestureRequiredError(error)) { | ||
throw error; | ||
} | ||
|
||
|
@@ -137,6 +157,18 @@ export async function showSidebar(): Promise<void> { | |
} | ||
} | ||
|
||
/** | ||
* Attach the sidebar to the page if it's not already attached. Safe to call from any frame. Resolves when the | ||
* sidebar is initialized. | ||
* @see showSidebarInTopFrame | ||
*/ | ||
export async function showSidebar(): Promise<void> { | ||
// Could consider explicitly calling showSidebarInTopFrame directly if we're already in the top frame. | ||
// But the messenger will already handle that case automatically. | ||
const topLevelFrame = await getTopLevelFrame(); | ||
await contentScriptApi.showSidebar(topLevelFrame); | ||
} | ||
|
||
/** | ||
* Force-show the panel for the given extension id | ||
* @param extensionId the extension UUID | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
nit: I think you should be able to get rid of the
waitForTimeout
by extending the timeout in thetoBeVisible
call.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.
In this case, I prefer the waitForTimeout there's an explicit sleep/wait in the mod. So it's matching that wait/sleep brick