Disallow users to set theme as a project-level setting (#3312)

* Disallow users to set theme as a project-level setting

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu)

* Fix up tests that assumed theme could be set at project level

* Missed two more tests that assumed theme was a project-level setting

---------

Co-authored-by: Jess Frazelle <jessfraz@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This commit is contained in:
Frank Noirot
2024-08-12 15:18:33 -04:00
committed by GitHub
parent 57a91cdb26
commit fb57df2cad
5 changed files with 98 additions and 138 deletions

View File

@ -98,14 +98,16 @@ const extrude001 = extrude(-10, sketch001)`
const commandBarButton = page.getByRole('button', { name: 'Commands' }) const commandBarButton = page.getByRole('button', { name: 'Commands' })
const cmdSearchBar = page.getByPlaceholder('Search commands') const cmdSearchBar = page.getByPlaceholder('Search commands')
const themeOption = page.getByRole('option', { const commandName = 'debug panel'
name: 'theme', const commandOption = page.getByRole('option', {
name: commandName,
exact: false, exact: false,
}) })
const commandLevelArgButton = page.getByRole('button', { name: 'level' }) const commandLevelArgButton = page.getByRole('button', { name: 'level' })
const commandThemeArgButton = page.getByRole('button', { name: 'value' }) const commandThemeArgButton = page.getByRole('button', { name: 'value' })
const paneSelector = page.getByRole('button', { name: 'debug panel' })
// This selector changes after we set the setting // This selector changes after we set the setting
let commandOptionInput = page.getByPlaceholder('Select an option') let commandOptionInput = page.getByPlaceholder('On')
await expect( await expect(
page.getByRole('button', { name: 'Start Sketch' }) page.getByRole('button', { name: 'Start Sketch' })
@ -127,17 +129,16 @@ const extrude001 = extrude(-10, sketch001)`
await expect(cmdSearchBar).toBeFocused() await expect(cmdSearchBar).toBeFocused()
// Try typing in the command bar // Try typing in the command bar
await cmdSearchBar.fill('theme') await cmdSearchBar.fill(commandName)
await expect(themeOption).toBeVisible() await expect(commandOption).toBeVisible()
await themeOption.click() await commandOption.click()
const themeInput = page.getByPlaceholder('Select an option') const toggleInput = page.getByPlaceholder('On')
await expect(themeInput).toBeVisible() await expect(toggleInput).toBeVisible()
await expect(themeInput).toBeFocused() await expect(toggleInput).toBeFocused()
// Select dark theme // Select On
await page.keyboard.press('ArrowDown') await page.keyboard.press('ArrowDown')
await page.keyboard.press('ArrowDown') await page.keyboard.press('ArrowDown')
await page.keyboard.press('ArrowDown') await expect(page.getByRole('option', { name: 'Off' })).toHaveAttribute(
await expect(page.getByRole('option', { name: 'system' })).toHaveAttribute(
'data-headlessui-state', 'data-headlessui-state',
'active' 'active'
) )
@ -145,21 +146,21 @@ const extrude001 = extrude(-10, sketch001)`
// Check the toast appeared // Check the toast appeared
await expect( await expect(
page.getByText(`Set theme to "system" for this project`) page.getByText(`Set show debug panel to "false" for this project`)
).toBeVisible() ).toBeVisible()
// Check that the theme changed // Check that the visibility changed
await expect(page.locator('body')).not.toHaveClass(`body-bg dark`) await expect(paneSelector).not.toBeVisible()
commandOptionInput = page.getByPlaceholder('system') commandOptionInput = page.getByPlaceholder('off')
// Test case for https://github.com/KittyCAD/modeling-app/issues/2882 // Test case for https://github.com/KittyCAD/modeling-app/issues/2882
await commandBarButton.click() await commandBarButton.click()
await cmdSearchBar.focus() await cmdSearchBar.focus()
await cmdSearchBar.fill('theme') await cmdSearchBar.fill(commandName)
await themeOption.click() await commandOption.click()
await expect(commandThemeArgButton).toBeDisabled() await expect(commandThemeArgButton).toBeDisabled()
await commandOptionInput.focus() await commandOptionInput.focus()
await commandOptionInput.fill('lig') await commandOptionInput.fill('on')
await commandLevelArgButton.click() await commandLevelArgButton.click()
await expect(commandLevelArgButton).toBeDisabled() await expect(commandLevelArgButton).toBeDisabled()
@ -197,7 +198,7 @@ const extrude001 = extrude(-10, sketch001)`
}) })
await expect(themeOption).toBeVisible() await expect(themeOption).toBeVisible()
await themeOption.click() await themeOption.click()
const themeInput = page.getByPlaceholder('Select an option') const themeInput = page.getByPlaceholder('dark')
await expect(themeInput).toBeVisible() await expect(themeInput).toBeVisible()
await expect(themeInput).toBeFocused() await expect(themeInput).toBeFocused()
// Select dark theme // Select dark theme
@ -212,7 +213,7 @@ const extrude001 = extrude(-10, sketch001)`
// Check the toast appeared // Check the toast appeared
await expect( await expect(
page.getByText(`Set theme to "system" for this project`) page.getByText(`Set theme to "system" as a user default`)
).toBeVisible() ).toBeVisible()
// Check that the theme changed // Check that the theme changed
await expect(page.locator('body')).not.toHaveClass(`body-bg dark`) await expect(page.locator('body')).not.toHaveClass(`body-bg dark`)

Binary file not shown.

Before

Width:  |  Height:  |  Size: 49 KiB

After

Width:  |  Height:  |  Size: 49 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 59 KiB

After

Width:  |  Height:  |  Size: 59 KiB

View File

@ -64,95 +64,55 @@ test.describe('Testing settings', () => {
.getByRole('button', { name: 'Start Sketch' }) .getByRole('button', { name: 'Start Sketch' })
.waitFor({ state: 'visible' }) .waitFor({ state: 'visible' })
const paneButtonLocator = page.getByTestId('debug-pane-button')
const headingLocator = page.getByRole('heading', {
name: 'Settings',
exact: true,
})
const inputLocator = page.locator('input[name="modeling-showDebugPanel"]')
// Open the settings modal with the browser keyboard shortcut // Open the settings modal with the browser keyboard shortcut
await page.keyboard.press('Meta+Shift+,') await page.keyboard.press('Meta+Shift+,')
await expect( await expect(headingLocator).toBeVisible()
page.getByRole('heading', { name: 'Settings', exact: true }) await page.locator('#showDebugPanel').getByText('OffOn').click()
).toBeVisible()
await page
.locator('select[name="app-theme"]')
.selectOption({ value: 'light' })
// Verify the toast appeared
await expect(
page.getByText(`Set theme to "light" for this project`)
).toBeVisible()
// Check that the theme changed
await expect(page.locator('body')).not.toHaveClass(`body-bg dark`)
// Check that the user setting was not changed
await page.getByRole('radio', { name: 'User' }).click()
await expect(page.locator('select[name="app-theme"]')).toHaveValue('dark')
// Roll back to default "system" theme
await page
.getByText(
'themeRoll back themeRoll back to match defaultThe overall appearance of the appl'
)
.hover()
await page
.getByRole('button', {
name: 'Roll back theme',
})
.click()
await expect(page.locator('select[name="app-theme"]')).toHaveValue('system')
// Check that the project setting did not change
await page.getByRole('radio', { name: 'Project' }).click()
await expect(page.locator('select[name="app-theme"]')).toHaveValue('light')
})
test('Project settings can be opened with keybinding from the editor', async ({
page,
}) => {
const u = await getUtils(page)
await page.setViewportSize({ width: 1200, height: 500 })
await u.waitForAuthSkipAppStart()
await page
.getByRole('button', { name: 'Start Sketch' })
.waitFor({ state: 'visible' })
// Close it and open again with keyboard shortcut, while KCL editor is focused
// Put the cursor in the editor // Put the cursor in the editor
await test.step('Open settings with keyboard shortcut', async () => {
await page.getByTestId('settings-close-button').click()
await page.locator('.cm-content').click() await page.locator('.cm-content').click()
// Open the settings modal with the browser keyboard shortcut
await page.keyboard.press('Meta+Shift+,') await page.keyboard.press('Meta+Shift+,')
await expect(headingLocator).toBeVisible()
await expect( })
page.getByRole('heading', { name: 'Settings', exact: true })
).toBeVisible()
await page
.locator('select[name="app-theme"]')
.selectOption({ value: 'light' })
// Verify the toast appeared // Verify the toast appeared
await expect( await expect(
page.getByText(`Set theme to "light" for this project`) page.getByText(`Set show debug panel to "false" for this project`)
).toBeVisible() ).toBeVisible()
// Check that the theme changed // Check that the theme changed
await expect(page.locator('body')).not.toHaveClass(`body-bg dark`) await expect(paneButtonLocator).not.toBeVisible()
// Check that the user setting was not changed // Check that the user setting was not changed
await page.getByRole('radio', { name: 'User' }).click() await page.getByRole('radio', { name: 'User' }).click()
await expect(page.locator('select[name="app-theme"]')).toHaveValue('dark') await expect(inputLocator).toBeChecked()
// Roll back to default "system" theme // Roll back to default of "off"
await page await await page
.getByText( .getByText('show debug panelRoll back show debug panelRoll back to match')
'themeRoll back themeRoll back to match defaultThe overall appearance of the appl'
)
.hover() .hover()
await page await page
.getByRole('button', { .getByRole('button', {
name: 'Roll back theme', name: 'Roll back show debug panel',
}) })
.click() .click()
await expect(page.locator('select[name="app-theme"]')).toHaveValue('system') await expect(inputLocator).not.toBeChecked()
// Check that the project setting did not change // Check that the project setting did not change
await page.getByRole('radio', { name: 'Project' }).click() await page.getByRole('radio', { name: 'Project' }).click()
await expect(page.locator('select[name="app-theme"]')).toHaveValue('light') await expect(
page.locator('input[name="modeling-showDebugPanel"]')
).not.toBeChecked()
}) })
test('Project and user settings can be reset', async ({ page }) => { test('Project and user settings can be reset', async ({ page }) => {
@ -163,69 +123,67 @@ test.describe('Testing settings', () => {
.getByRole('button', { name: 'Start Sketch' }) .getByRole('button', { name: 'Start Sketch' })
.waitFor({ state: 'visible' }) .waitFor({ state: 'visible' })
// Put the cursor in the editor const projectSettingsTab = page.getByRole('radio', { name: 'Project' })
await page.locator('.cm-content').click() const userSettingsTab = page.getByRole('radio', { name: 'User' })
const resetButton = page.getByRole('button', {
// Open the settings modal with the browser keyboard shortcut name: 'Restore default settings',
await page.keyboard.press('Meta+Shift+,') })
const themeColorSetting = page.locator('#themeColor').getByRole('slider')
const settingValues = {
default: '259',
user: '120',
project: '50',
}
// Open the settings modal with lower-right button
await page.getByRole('link', { name: 'Settings' }).last().click()
await expect( await expect(
page.getByRole('heading', { name: 'Settings', exact: true }) page.getByRole('heading', { name: 'Settings', exact: true })
).toBeVisible() ).toBeVisible()
await test.step('Set up theme color', async () => {
// Verify we're looking at the project-level settings,
// and it's set to default value
await expect(projectSettingsTab).toBeChecked()
await expect(themeColorSetting).toHaveValue(settingValues.default)
// Set project-level value to 50
await themeColorSetting.fill(settingValues.project)
// Set user-level value to 120
await userSettingsTab.click()
await themeColorSetting.fill(settingValues.user)
await projectSettingsTab.click()
})
await test.step('Reset project settings', async () => {
// Click the reset settings button. // Click the reset settings button.
await page.getByRole('button', { name: 'Restore default settings' }).click() await resetButton.click()
await page // Verify it is now set to the inherited user value
.locator('select[name="app-theme"]') await expect(themeColorSetting).toHaveValue(settingValues.default)
.selectOption({ value: 'light' })
// Verify the toast appeared // Check that the user setting also rolled back
await expect( await userSettingsTab.click()
page.getByText(`Set theme to "light" for this project`) await expect(themeColorSetting).toHaveValue(settingValues.default)
).toBeVisible() await projectSettingsTab.click()
// Check that the theme changed
await expect(page.locator('body')).not.toHaveClass(`body-bg dark`)
await expect(page.locator('select[name="app-theme"]')).toHaveValue('light')
// Check that the user setting was not changed // Set project-level value to 50 again to test the user-level reset
await page.getByRole('radio', { name: 'User' }).click() await themeColorSetting.fill(settingValues.project)
await expect(page.locator('select[name="app-theme"]')).toHaveValue('system') await userSettingsTab.click()
})
// Click the reset settings button. await test.step('Reset user settings', async () => {
await page.getByRole('button', { name: 'Restore default settings' }).click() // Change the setting and click the reset settings button.
await themeColorSetting.fill(settingValues.user)
await resetButton.click()
// Verify it is now set to the default value // Verify it is now set to the default value
await expect(page.locator('select[name="app-theme"]')).toHaveValue('system') await expect(themeColorSetting).toHaveValue(settingValues.default)
// Set the user theme to light. // Check that the project setting also changed
await page await projectSettingsTab.click()
.locator('select[name="app-theme"]') await expect(themeColorSetting).toHaveValue(settingValues.default)
.selectOption({ value: 'light' }) })
// Verify the toast appeared
await expect(
page.getByText(`Set theme to "light" as a user default`)
).toBeVisible()
// Check that the theme changed
await expect(page.locator('body')).not.toHaveClass(`body-bg dark`)
await expect(page.locator('select[name="app-theme"]')).toHaveValue('light')
await page.getByRole('radio', { name: 'Project' }).click()
await expect(page.locator('select[name="app-theme"]')).toHaveValue('light')
// Click the reset settings button.
await page.getByRole('button', { name: 'Restore default settings' }).click()
// Verify it is now set to the default value
await expect(page.locator('select[name="app-theme"]')).toHaveValue('system')
await page.getByRole('radio', { name: 'User' }).click()
await expect(page.locator('select[name="app-theme"]')).toHaveValue('system')
// Click the reset settings button.
await page.getByRole('button', { name: 'Restore default settings' }).click()
// Verify it is now set to the default value
await expect(page.locator('select[name="app-theme"]')).toHaveValue('system')
}) })
}) })

View File

@ -114,6 +114,7 @@ export function createSettings() {
* The overall appearance of the app: light, dark, or system * The overall appearance of the app: light, dark, or system
*/ */
theme: new Setting<Themes>({ theme: new Setting<Themes>({
hideOnLevel: 'project',
defaultValue: Themes.System, defaultValue: Themes.System,
description: 'The overall appearance of the app', description: 'The overall appearance of the app',
validate: (v) => isEnumMember(v, Themes), validate: (v) => isEnumMember(v, Themes),