From 292f89859f7c4d0674444c4ae21ebfd8f32dbc83 Mon Sep 17 00:00:00 2001 From: Frank Noirot Date: Wed, 11 Sep 2024 09:39:10 -0400 Subject: [PATCH] Make settings reset button only reset current level (#3855) * Update test to expect new behavior (failing) * Update behavior to match new test expectations * Make reset button more clear * Fix eslint issue * Fix up separate test that relied on old reset logic --- e2e/playwright/testing-settings.spec.ts | 121 ++++++++++-------- src/components/Settings/AllSettingsFields.tsx | 14 +- src/machines/settingsMachine.ts | 31 +++-- 3 files changed, 96 insertions(+), 70 deletions(-) diff --git a/e2e/playwright/testing-settings.spec.ts b/e2e/playwright/testing-settings.spec.ts index df273b05e..6ba9e5de3 100644 --- a/e2e/playwright/testing-settings.spec.ts +++ b/e2e/playwright/testing-settings.spec.ts @@ -8,7 +8,7 @@ import { tearDown, executorInputPath, } from './test-utils' -import { SaveSettingsPayload } from 'lib/settings/settingsTypes' +import { SaveSettingsPayload, SettingsLevel } from 'lib/settings/settingsTypes' import { TEST_SETTINGS_KEY, TEST_SETTINGS_CORRUPTED } from './storageStates' import * as TOML from '@iarna/toml' @@ -154,29 +154,33 @@ test.describe('Testing settings', () => { test('Project and user settings can be reset', 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' }) + await test.step(`Setup`, async () => { + await page.setViewportSize({ width: 1200, height: 500 }) + await u.waitForAuthSkipAppStart() + }) + // Selectors and constants const projectSettingsTab = page.getByRole('radio', { name: 'Project' }) const userSettingsTab = page.getByRole('radio', { name: 'User' }) - const resetButton = page.getByRole('button', { - name: 'Restore default settings', - }) + const resetButton = (level: SettingsLevel) => + page.getByRole('button', { + name: `Reset ${level}-level settings`, + }) const themeColorSetting = page.locator('#themeColor').getByRole('slider') const settingValues = { default: '259', user: '120', project: '50', } + const resetToast = (level: SettingsLevel) => + page.getByText(`${level}-level settings were reset`) - // Open the settings modal with lower-right button - await page.getByRole('link', { name: 'Settings' }).last().click() - await expect( - page.getByRole('heading', { name: 'Settings', exact: true }) - ).toBeVisible() + await test.step(`Open the settings modal`, async () => { + await page.getByRole('link', { name: 'Settings' }).last().click() + await expect( + page.getByRole('heading', { name: 'Settings', exact: true }) + ).toBeVisible() + }) await test.step('Set up theme color', async () => { // Verify we're looking at the project-level settings, @@ -195,37 +199,40 @@ test.describe('Testing settings', () => { await test.step('Reset project settings', async () => { // Click the reset settings button. - await resetButton.click() + await resetButton('project').click() - await expect(page.getByText('Settings restored to default')).toBeVisible() - await expect( - page.getByText('Settings restored to default') - ).not.toBeVisible() + await expect(resetToast('project')).toBeVisible() + await expect(resetToast('project')).not.toBeVisible() // Verify it is now set to the inherited user value - await expect(themeColorSetting).toHaveValue(settingValues.default) + await expect(themeColorSetting).toHaveValue(settingValues.user) - // Check that the user setting also rolled back - await userSettingsTab.click() - await expect(themeColorSetting).toHaveValue(settingValues.default) - await projectSettingsTab.click() + await test.step(`Check that the user settings did not change`, async () => { + await userSettingsTab.click() + await expect(themeColorSetting).toHaveValue(settingValues.user) + }) - // Set project-level value to 50 again to test the user-level reset - await themeColorSetting.fill(settingValues.project) - await userSettingsTab.click() + await test.step(`Set project-level again to test the user-level reset`, async () => { + await projectSettingsTab.click() + await themeColorSetting.fill(settingValues.project) + await userSettingsTab.click() + }) }) await test.step('Reset user settings', async () => { - // Change the setting and click the reset settings button. - await themeColorSetting.fill(settingValues.user) - await resetButton.click() + // Click the reset settings button. + await resetButton('user').click() + + await expect(resetToast('user')).toBeVisible() + await expect(resetToast('user')).not.toBeVisible() // Verify it is now set to the default value await expect(themeColorSetting).toHaveValue(settingValues.default) - // Check that the project setting also changed - await projectSettingsTab.click() - await expect(themeColorSetting).toHaveValue(settingValues.default) + await test.step(`Check that the project settings did not change`, async () => { + await projectSettingsTab.click() + await expect(themeColorSetting).toHaveValue(settingValues.project) + }) }) }) @@ -429,25 +436,37 @@ test.describe('Testing settings', () => { test('Changing modeling default unit', 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' }) - - const userSettingsTab = page.getByRole('radio', { name: 'User' }) - - // Open the settings modal with lower-right button - await page.getByRole('link', { name: 'Settings' }).last().click() - await expect( - page.getByRole('heading', { name: 'Settings', exact: true }) - ).toBeVisible() - - const resetButton = page.getByRole('button', { - name: 'Restore default settings', + await test.step(`Test setup`, async () => { + await page.setViewportSize({ width: 1200, height: 500 }) + await u.waitForAuthSkipAppStart() + await page + .getByRole('button', { name: 'Start Sketch' }) + .waitFor({ state: 'visible' }) + }) + + // Selectors and constants + const userSettingsTab = page.getByRole('radio', { name: 'User' }) + const projectSettingsTab = page.getByRole('radio', { name: 'Project' }) + const defaultUnitSection = page.getByText( + 'default unitRoll back default unitRoll back to match' + ) + const defaultUnitRollbackButton = page.getByRole('button', { + name: 'Roll back default unit', + }) + + await test.step(`Open the settings modal`, async () => { + await page.getByRole('link', { name: 'Settings' }).last().click() + await expect( + page.getByRole('heading', { name: 'Settings', exact: true }) + ).toBeVisible() + }) + + await test.step(`Reset unit setting`, async () => { + await userSettingsTab.click() + await defaultUnitSection.hover() + await defaultUnitRollbackButton.click() + await projectSettingsTab.click() }) - // Default unit should be mm - await resetButton.click() await test.step('Change modeling default unit within project tab', async () => { const changeUnitOfMeasureInProjectTab = async (unitOfMeasure: string) => { diff --git a/src/components/Settings/AllSettingsFields.tsx b/src/components/Settings/AllSettingsFields.tsx index 33da3d31e..480223367 100644 --- a/src/components/Settings/AllSettingsFields.tsx +++ b/src/components/Settings/AllSettingsFields.tsx @@ -12,7 +12,6 @@ import { useLocation, useNavigate } from 'react-router-dom' import { isDesktop } from 'lib/isDesktop' import { ActionButton } from 'components/ActionButton' import { SettingsFieldInput } from './SettingsFieldInput' -import { getInitialDefaultDir } from 'lib/desktop' import toast from 'react-hot-toast' import { APP_VERSION } from 'routes/Settings' import { PATHS } from 'lib/paths' @@ -214,14 +213,15 @@ export const AllSettingsFields = forwardRef( )} { - const defaultDirectory = await getInitialDefaultDir() + onClick={() => { send({ type: 'Reset settings', - defaultDirectory, + level: searchParamTab, }) - toast.success('Settings restored to default') - }, reportRejection)} + toast.success( + `Your ${searchParamTab}-level settings were reset` + ) + }} iconStart={{ icon: 'refresh', size: 'sm', @@ -229,7 +229,7 @@ export const AllSettingsFields = forwardRef( bgClassName: 'bg-destroy-70', }} > - Restore default settings + Reset {searchParamTab}-level settings diff --git a/src/machines/settingsMachine.ts b/src/machines/settingsMachine.ts index 24ceaea2f..5409c8e3c 100644 --- a/src/machines/settingsMachine.ts +++ b/src/machines/settingsMachine.ts @@ -8,6 +8,11 @@ import { SettingsPaths, WildcardSetEvent, } from 'lib/settings/settingsTypes' +import { + configurationToSettingsPayload, + projectConfigurationToSettingsPayload, + setSettingsAtLevel, +} from 'lib/settings/settingsUtils' export const settingsMachine = setup({ types: { @@ -24,7 +29,10 @@ export const settingsMachine = setup({ type: 'set.modeling.units' data: { level: SettingsLevel; value: BaseUnit } } - | { type: 'Reset settings'; defaultDirectory: string } + | { + type: 'Reset settings' + level: SettingsLevel + } | { type: 'Set all settings'; settings: typeof settings }, }, actions: { @@ -37,17 +45,16 @@ export const settingsMachine = setup({ setClientSideSceneUnits: () => {}, persistSettings: () => {}, resetSettings: assign(({ context, event }) => { - if (!('defaultDirectory' in event)) return {} - // Reset everything except onboarding status, - // which should be preserved - const newSettings = createSettings() - if (context.app.onboardingStatus.user) { - newSettings.app.onboardingStatus.user = - context.app.onboardingStatus.user - } - // We instead pass in the default directory since it's asynchronous - // to re-initialize, and that can be done by the caller. - newSettings.app.projectDirectory.default = event.defaultDirectory + if (!('level' in event)) return {} + + // Create a new, blank payload + const newPayload = + event.level === 'user' + ? configurationToSettingsPayload({}) + : projectConfigurationToSettingsPayload({}) + + // Reset the settings at that level + const newSettings = setSettingsAtLevel(context, event.level, newPayload) return newSettings }),