Skip to content
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

Edit text for "delete asset" modal #11857

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
4 changes: 3 additions & 1 deletion app/common/src/text/english.json
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@
"thisFolderFailedToFetch": "This folder failed to fetch.",
"yourTrashIsEmpty": "Your trash is empty.",
"deleteTheAssetTypeTitle": "delete the $0 '$1'",
"deleteTheAssetTypeTitleForever": "permanently delete the $0 '$1'",
"trashTheAssetTypeTitle": "move the $0 '$1' to Trash",
"notImplemetedYet": "Not implemented yet.",
"newLabelButtonLabel": "New label",
Expand Down Expand Up @@ -456,7 +457,8 @@
"youHaveNoRecentProjects": "You have no recent projects. Switch to another category to create a project.",
"youHaveNoFiles": "This folder is empty. You can create a project using the buttons above.",
"placeholderChatPrompt": "Login or register to access live chat with our support team.",
"confirmPrompt": "Are you sure you want to $0?",
"confirmPrompt": "Do you really want to $0?",
"thisOperationCannotBeUndone": "This operation is final and cannot be undone.",
"couldNotInviteUser": "Could not invite user $0",
"inviteFormSeatsLeft": "You have $0 seats left on your plan. Upgrade to invite more",
"inviteFormSeatsLeftError": "You have exceed the number of seats on your plan by $0",
Expand Down
5 changes: 1 addition & 4 deletions app/common/src/text/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@
import { unsafeKeys } from '../utilities/data/object'
import ENGLISH from './english.json' with { type: 'json' }

// =============
// === Types ===
// =============

/** Possible languages in which to display text. */
export enum Language {
english = 'english',
Expand Down Expand Up @@ -46,6 +42,7 @@ interface PlaceholderOverrides {
readonly confirmPrompt: [action: string]
readonly trashTheAssetTypeTitle: [assetType: string, assetName: string]
readonly deleteTheAssetTypeTitle: [assetType: string, assetName: string]
readonly deleteTheAssetTypeTitleForever: [assetType: string, assetName: string]
readonly couldNotInviteUser: [userEmail: string]
readonly filesWithoutConflicts: [fileCount: number]
readonly projectsWithoutConflicts: [projectCount: number]
Expand Down
73 changes: 63 additions & 10 deletions app/gui/integration-test/dashboard/actions/BaseActions.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/** @file The base class from which all `Actions` classes are derived. */
import { expect, test, type Locator, type Page } from '@playwright/test'

import type { AutocompleteKeybind } from '#/utilities/inputBindings'
import type { AutocompleteKeybind, ModifierKey } from '#/utilities/inputBindings'

/** `Meta` (`Cmd`) on macOS, and `Control` on all other platforms. */
async function modModifier(page: Page) {
export async function modModifier(page: Page) {
let userAgent = ''
await test.step('Detect browser OS', async () => {
userAgent = await page.evaluate(() => navigator.userAgent)
Expand Down Expand Up @@ -51,11 +51,17 @@ export default class BaseActions<Context> implements Promise<void> {
}

/**
* Press a key, replacing the text `Mod` with `Meta` (`Cmd`) on macOS, and `Control`
* on all other platforms.
* Return the appropriate key for a shortcut, replacing the text `Mod` with `Meta` (`Cmd`) on macOS,
* and `Control` on all other platforms. Similarly, replace the text `Delete` with `Backspace`
* on `macOS`, and `Delete` on all other platforms.
*/
static press(page: Page, keyOrShortcut: string): Promise<void> {
return test.step(`Press '${keyOrShortcut}'`, async () => {
static async withNormalizedKey(
page: Page,
keyOrShortcut: string,
callback: (shortcut: string) => Promise<void>,
description = 'Normalize',
): Promise<void> {
return test.step(`${description} '${keyOrShortcut}'`, async () => {
if (/\bMod\b|\bDelete\b/.test(keyOrShortcut)) {
let userAgent = ''
await test.step('Detect browser OS', async () => {
Expand All @@ -65,13 +71,23 @@ export default class BaseActions<Context> implements Promise<void> {
const ctrlKey = isMacOS ? 'Meta' : 'Control'
const deleteKey = isMacOS ? 'Backspace' : 'Delete'
const shortcut = keyOrShortcut.replace(/\bMod\b/, ctrlKey).replace(/\bDelete\b/, deleteKey)
await page.keyboard.press(shortcut)
return await callback(shortcut)
} else {
await page.keyboard.press(keyOrShortcut)
return callback(keyOrShortcut)
}
})
}

/** Press a key or shortcut. */
static async press(page: Page, keyOrShortcut: string) {
await BaseActions.withNormalizedKey(
page,
keyOrShortcut,
(shortcut) => page.keyboard.press(shortcut),
'Press and release',
)
}

/** Proxies the `then` method of the internal {@link Promise}. */
async then<T, E>(
onfulfilled?: (() => PromiseLike<T> | T) | null | undefined,
Expand Down Expand Up @@ -135,8 +151,45 @@ export default class BaseActions<Context> implements Promise<void> {
* Press a key, replacing the text `Mod` with `Meta` (`Cmd`) on macOS, and `Control`
* on all other platforms.
*/
press<Key extends string>(keyOrShortcut: AutocompleteKeybind<Key>) {
return this.do((page) => BaseActions.press(page, keyOrShortcut))
press<Key extends string>(keyOrShortcut: AutocompleteKeybind<Key> | ModifierKey) {
return this.do((page) =>
BaseActions.withNormalizedKey(
page,
keyOrShortcut,
(shortcut) => page.keyboard.press(shortcut),
'Press and release',
),
)
}

/**
* Press a key, replacing the text `Mod` with `Meta` (`Cmd`) on macOS, and `Control`
* on all other platforms.
*/
down<Key extends string>(keyOrShortcut: AutocompleteKeybind<Key> | ModifierKey) {
return this.do((page) =>
BaseActions.withNormalizedKey(
page,
keyOrShortcut,
(shortcut) => page.keyboard.down(shortcut),
'Press',
),
)
}

/**
* Press a key, replacing the text `Mod` with `Meta` (`Cmd`) on macOS, and `Control`
* on all other platforms.
*/
up<Key extends string>(keyOrShortcut: AutocompleteKeybind<Key> | ModifierKey) {
return this.do((page) =>
BaseActions.withNormalizedKey(
page,
keyOrShortcut,
(shortcut) => page.keyboard.up(shortcut),
'Release',
),
)
}

/** Perform actions until a predicate passes. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,14 @@ export default class DrivePageActions<Context> extends PageActions<Context> {
})
}

/** Clear trash. */
clearTrash() {
return this.step('Clear trash', async (page) => {
await page.getByText(TEXT.clearTrash).click()
await page.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
})
}

/** Create a new empty project. */
newEmptyProject() {
return this.step(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export interface ContextMenuActions<T extends BaseActions<Context>, Context> {
readonly snapshot: () => T
readonly moveNonFolderToTrash: () => T
readonly moveFolderToTrash: () => T
readonly moveAllToTrash: () => T
readonly moveAllToTrash: (confirm?: boolean) => T
readonly restoreFromTrash: () => T
readonly restoreAllFromTrash: () => T
readonly share: () => T
Expand Down Expand Up @@ -77,13 +77,16 @@ export function contextMenuActions<T extends BaseActions<Context>, Context>(
// Confirm the deletion in the dialog
await page.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
}),
moveAllToTrash: () =>
step('Move all to trash (context menu)', (page) =>
page
moveAllToTrash: (hasFolder = false) =>
step('Move all to trash (context menu)', async (page) => {
await page
.getByRole('button', { name: TEXT.moveAllToTrashShortcut })
.getByText(TEXT.moveAllToTrashShortcut)
.click(),
),
.click()
if (hasFolder) {
await page.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
}
}),
restoreFromTrash: () =>
step('Restore from trash (context menu)', (page) =>
page
Expand Down
39 changes: 39 additions & 0 deletions app/gui/integration-test/dashboard/delete.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/** @file Test copying, moving, cutting and pasting. */
import { expect, test } from '@playwright/test'

import { modModifier } from 'integration-test/dashboard/actions/BaseActions'
import { mockAllAndLogin, TEXT } from './actions'

test('delete and restore', ({ page }) =>
Expand Down Expand Up @@ -54,3 +55,41 @@ test('delete and restore (keyboard)', ({ page }) =>
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(1)
}))

test('clear trash', ({ page }) =>
mockAllAndLogin({ page })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can just setup the folders/projects using setupAPI here? This is faster and creating assets already tested in the other tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgor! nice catch

.createFolder()
.createFolder()
.createFolder()
.newEmptyProject()
.newEmptyProject()
.newEmptyProject()
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(6)
})
.driveTable.withRows(async (rows, _nonRows, _context, page) => {
const mod = await modModifier(page)
// Parallelizing this using `Promise.all` makes it inconsistent.
const rowEls = await rows.all()
for (const row of rowEls) {
await row.click({ modifiers: [mod] })
}
})
.driveTable.rightClickRow(0)
.contextMenu.moveAllToTrash(true)
.driveTable.expectPlaceholderRow()
.goToCategory.trash()
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(6)
})
.clearTrash()
.driveTable.expectTrashPlaceholderRow()
.goToCategory.cloud()
.expectStartModal()
.withStartModal(async (startModal) => {
await expect(startModal).toBeVisible()
})
.close()
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(1)
}))
3 changes: 2 additions & 1 deletion app/gui/src/dashboard/layouts/AssetContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,8 @@ export default function AssetContextMenu(props: AssetContextMenuProps) {
setModal(
<ConfirmDeleteModal
defaultOpen
actionText={`delete the ${asset.type} '${asset.title}' forever`}
cannotUndo
actionText={getText('deleteTheAssetTypeTitleForever', asset.type, asset.title)}
doDelete={() => {
const ids = new Set([asset.id])
dispatchAssetEvent({ type: AssetEventType.deleteForever, ids })
Expand Down
2 changes: 1 addition & 1 deletion app/gui/src/dashboard/layouts/AssetsTableContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export default function AssetsTableContextMenu(props: AssetsTableContextMenuProp

if (category.type === 'trash') {
return (
selectedKeys.size !== 0 && (
selectedKeys.size > 1 && (
<ContextMenu
aria-label={getText('assetsTableContextMenuLabel')}
hidden={hidden}
Expand Down
4 changes: 3 additions & 1 deletion app/gui/src/dashboard/layouts/DriveBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,9 @@ export default function DriveBar(props: DriveBarProps) {

<ConfirmDeleteModal
actionText={getText('allTrashedItemsForever')}
doDelete={doEmptyTrash}
doDelete={() => {
doEmptyTrash()
}}
/>
</DialogTrigger>
{pasteDataStatus}
Expand Down
19 changes: 16 additions & 3 deletions app/gui/src/dashboard/modals/ConfirmDeleteModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,25 @@ import { useText } from '#/providers/TextProvider'

/** Props for a {@link ConfirmDeleteModal}. */
export interface ConfirmDeleteModalProps {
readonly defaultOpen?: boolean
readonly defaultOpen?: boolean | undefined
readonly cannotUndo?: boolean | undefined
/** Must fit in the sentence "Are you sure you want to <action>?". */
readonly actionText: string
/** The label shown on the colored confirmation button. "Delete" by default. */
readonly actionButtonLabel?: string
readonly actionButtonLabel?: string | undefined
readonly doDelete: () => void
}

/** A modal for confirming the deletion of an asset. */
export default function ConfirmDeleteModal(props: ConfirmDeleteModalProps) {
const { defaultOpen, actionText, actionButtonLabel = 'Delete', doDelete } = props
const {
// MUST NOT be defaulted. Omitting this value should fall back to `Dialog`'s behavior.
defaultOpen,
cannotUndo = false,
actionText,
actionButtonLabel = 'Delete',
doDelete,
} = props

const { unsetModal } = useSetModal()
const { getText } = useText()
Expand All @@ -34,6 +42,11 @@ export default function ConfirmDeleteModal(props: ConfirmDeleteModalProps) {
>
<Form schema={z.object({})} method="dialog" onSubmit={doDelete} onSubmitSuccess={unsetModal}>
<Text className="relative">{getText('confirmPrompt', actionText)}</Text>
{cannotUndo && (
<Text className="relative" weight="bold">
{getText('thisOperationCannotBeUndone')}
</Text>
)}

<ButtonGroup>
<Form.Submit variant="delete" className="relative" autoFocus>
Expand Down
Loading