Skip to content

Commit 1570865

Browse files
author
Shane Osbourne
committed
dont use message for opening links - detect custom scheme instead
1 parent 127474a commit 1570865

File tree

4 files changed

+18
-53
lines changed

4 files changed

+18
-53
lines changed

packages/special-pages/pages/duckplayer/src/js/index.js

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,20 @@
2525
* On Page Load
2626
* - {@link DuckPlayerPageMessages.onUserValuesChanged} begins immediately. It expects an initial value, and then will continue to listen for updates
2727
*
28-
* Then the following 2 messages can be sent at any time
28+
* Then the following message can be sent at any time
2929
* - {@link DuckPlayerPageMessages.setUserValues}
30-
* - {@link DuckPlayerPageMessages.openSettings}
3130
*
3231
* Please see {@link DuckPlayerPageMessages} for the up-to-date list
3332
*/
3433
import {
3534
Messaging,
3635
WindowsMessagingConfig,
37-
WebkitMessagingConfig,
3836
MessagingContext, TestTransportConfig
3937
} from '../../../../../messaging/index.js'
40-
import { DuckPlayerPageMessages, UserValues, OpenSettings } from './messages'
38+
import { DuckPlayerPageMessages, UserValues } from './messages'
4139

4240
// for docs
43-
export { DuckPlayerPageMessages, UserValues, OpenSettings }
41+
export { DuckPlayerPageMessages, UserValues }
4442

4543
const VideoPlayer = {
4644
/**
@@ -520,10 +518,9 @@ const Setting = {
520518
})
521519

522520
const settingsIcon = Setting.settingsIcon()
523-
settingsIcon.addEventListener('click', (e) => {
524-
e.preventDefault()
525-
Comms.messaging.openSettings(new OpenSettings({ target: 'duckplayer' }))
526-
})
521+
522+
// windows settings - we will need to alter for other platforms.
523+
settingsIcon.setAttribute('href', 'duck://settings/duckplayer')
527524
}
528525
}
529526

packages/special-pages/pages/duckplayer/src/js/messages.js

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,6 @@ export class DuckPlayerPageMessages {
3131
return this.messaging.request('getUserValues')
3232
}
3333

34-
/**
35-
* This occurs when a user has clicked on the 'Settings Cog'
36-
*
37-
* **OpenSettings example**
38-
* ```json
39-
* { "target": "duckplayer" }
40-
* ```
41-
*
42-
* @param {OpenSettings} openSettings
43-
*/
44-
openSettings (openSettings) {
45-
this.messaging.notify('openSettings', openSettings)
46-
}
47-
4834
/**
4935
* This is a subscription that we set up when the page loads.
5036
* We use this value to show/hide the checkboxes.
@@ -77,7 +63,7 @@ export class DuckPlayerPageMessages {
7763
}
7864

7965
/**
80-
* This data structure is sent to enable settings to be updated
66+
* This data structure is sent to enable user settings to be updated
8167
*
8268
* ```js
8369
* [[include:packages/special-pages/pages/duckplayer/src/js/messages.example.js]]```
@@ -105,13 +91,3 @@ export class UserValues {
10591
this.overlayInteracted = params.overlayInteracted
10692
}
10793
}
108-
109-
export class OpenSettings {
110-
/**
111-
* @param {object} params
112-
* @param {"duckplayer"} params.target
113-
*/
114-
constructor (params) {
115-
this.target = params.target
116-
}
117-
}

packages/special-pages/tests/duckplayer.spec.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,7 @@ test.describe('duckplayer toolbar', () => {
6060
await duckplayer.hasLoadedIframe()
6161

6262
await page.mouse.move(1, 1)
63-
await duckplayer.clickToOpenSettings()
64-
await duckplayer.didOpenSettingsInNewPage(page)
63+
await duckplayer.opensSettingsInNewTab()
6564
})
6665
test('opening in youtube', async ({ page }, workerInfo) => {
6766
const duckplayer = DuckPlayerPage.create(page, workerInfo)

packages/special-pages/tests/page-objects/duck-player.js

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -141,25 +141,18 @@ export class DuckPlayerPage {
141141
await expect(this.page.locator('.info-icon-tooltip')).toBeHidden()
142142
}
143143

144-
async clickToOpenSettings () {
144+
async opensSettingsInNewTab () {
145+
// duck:// scheme will fail, but we can assert that it was tried and grab the URL
146+
const failure = new Promise(resolve => {
147+
this.page.context().on('requestfailed', f => {
148+
resolve(f.url())
149+
})
150+
})
151+
145152
await this.page.locator('.open-settings').click()
146-
}
147153

148-
/**
149-
* @param {import("@playwright/test").Page} otherPage
150-
*/
151-
async didOpenSettingsInNewPage (otherPage) {
152-
const calls = await this.mocks.waitForCallCount({ method: 'openSettings', count: 1 })
153-
expect(calls[0]).toMatchObject({
154-
payload: {
155-
context: 'specialPages',
156-
featureName: 'duckPlayerPage',
157-
method: 'openSettings',
158-
params: {
159-
target: 'duckplayer'
160-
}
161-
}
162-
})
154+
// this is for windows, we'll need to support more
155+
expect(await failure).toEqual('duck://settings/duckplayer')
163156
}
164157

165158
async clickPlayOnYouTube () {

0 commit comments

Comments
 (0)