Skip to content

[refactor] Migrate SettingDialog to BaseModalLayout design system #8269

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.

Sign up for GitHub

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

Closed
viva-jinyi wants to merge 1 commit into from
Closed

[refactor] Migrate SettingDialog to BaseModalLayout design system #8269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions browser_tests/fixtures/components/SettingDialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ export class SettingDialog {
) {}

get root() {
return this.page.locator('div.settings-container')
return this.page.locator('[data-testid="settings-dialog"]')
}

async open() {
await this.comfyPage.executeCommand('Comfy.ShowSettingsDialog')
await this.page.waitForSelector('div.settings-container')
await this.page.waitForSelector('[data-testid="settings-dialog"]')
}

/**
Expand All @@ -23,9 +23,7 @@ export class SettingDialog {
* @param value - The value to set
*/
async setStringSetting(id: string, value: string) {
const settingInputDiv = this.page.locator(
`div.settings-container div[id="${id}"]`
)
const settingInputDiv = this.root.locator(`div[id="${id}"]`)
await settingInputDiv.locator('input').fill(value)
}

Expand All @@ -34,15 +32,15 @@ export class SettingDialog {
* @param id - The id of the setting
*/
async toggleBooleanSetting(id: string) {
const settingInputDiv = this.page.locator(
`div.settings-container div[id="${id}"]`
)
const settingInputDiv = this.root.locator(`div[id="${id}"]`)
await settingInputDiv.locator('input').click()
}

async goToAboutPanel() {
const aboutButton = this.page.locator('li[aria-label="About"]')
const aboutButton = this.root.locator('nav [role="button"]', {
hasText: 'About'
})
await aboutButton.click()
await this.page.waitForSelector('div.about-container')
await this.page.waitForSelector('.about-container')
}
}
23 changes: 17 additions & 6 deletions browser_tests/tests/dialog.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,13 @@ test.describe('Missing models warning', () => {
test.describe('Settings', () => {
test('@mobile Should be visible on mobile', async ({ comfyPage }) => {
await comfyPage.page.keyboard.press('Control+,')
const settingsContent = comfyPage.page.locator('.settings-content')
await expect(settingsContent).toBeVisible()
const isUsableHeight = await settingsContent.evaluate(
const settingsDialog = comfyPage.page.locator(
'[data-testid="settings-dialog"]'
)
await expect(settingsDialog).toBeVisible()
const contentArea = settingsDialog.locator('main')
await expect(contentArea).toBeVisible()
const isUsableHeight = await contentArea.evaluate(
(el) => el.clientHeight > 30
)
expect(isUsableHeight).toBeTruthy()
Expand All @@ -244,7 +248,9 @@ test.describe('Settings', () => {
await comfyPage.page.keyboard.down('ControlOrMeta')
await comfyPage.page.keyboard.press(',')
await comfyPage.page.keyboard.up('ControlOrMeta')
const settingsLocator = comfyPage.page.locator('.settings-container')
const settingsLocator = comfyPage.page.locator(
'[data-testid="settings-dialog"]'
)
await expect(settingsLocator).toBeVisible()
await comfyPage.page.keyboard.press('Escape')
await expect(settingsLocator).not.toBeVisible()
Expand All @@ -261,10 +267,15 @@ test.describe('Settings', () => {
test('Should persist keybinding setting', async ({ comfyPage }) => {
// Open the settings dialog
await comfyPage.page.keyboard.press('Control+,')
await comfyPage.page.waitForSelector('.settings-container')
await comfyPage.page.waitForSelector('[data-testid="settings-dialog"]')

// Open the keybinding tab
await comfyPage.page.getByLabel('Keybinding').click()
const settingsDialog = comfyPage.page.locator(
'[data-testid="settings-dialog"]'
)
await settingsDialog
.locator('nav [role="button"]', { hasText: 'Keybinding' })
.click()
await comfyPage.page.waitForSelector(
'[placeholder="Search Keybindings..."]'
)
Expand Down
4 changes: 2 additions & 2 deletions browser_tests/tests/subgraph.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ test.describe('Subgraph Operations', () => {

// Open settings dialog using hotkey
await comfyPage.page.keyboard.press('Control+,')
await comfyPage.page.waitForSelector('.settings-container', {
await comfyPage.page.waitForSelector('[data-testid="settings-dialog"]', {
state: 'visible'
})

Expand All @@ -804,7 +804,7 @@ test.describe('Subgraph Operations', () => {

// Dialog should be closed
await expect(
comfyPage.page.locator('.settings-container')
comfyPage.page.locator('[data-testid="settings-dialog"]')
).not.toBeVisible()

// Should still be in subgraph
Expand Down
Loading
Loading