Skip to content

fix(SNOTES-480): display tag title as string in delete dialog #2904

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion packages/utils/src/Domain/Utils/Utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { sanitize } from 'dompurify'
import { find, isArray, mergeWith, remove, uniq, uniqWith } from 'lodash'
import { escape, find, isArray, mergeWith, remove, uniq, uniqWith } from 'lodash'
import { AnyRecord } from '@standardnotes/common'

const collator = typeof Intl !== 'undefined' ? new Intl.Collator('en', { numeric: true }) : undefined
Expand Down Expand Up @@ -612,6 +612,10 @@ export function sanitizeHtmlString(html: string): string {
return sanitize(html)
}

export function escapeHtmlString(html: string): string {
return escape(html)
}

let sharedDateFormatter: unknown
export function dateToLocalizedString(date: Date): string {
if (typeof Intl !== 'undefined' && Intl.DateTimeFormat && typeof navigator !== 'undefined') {
Expand Down
23 changes: 17 additions & 6 deletions packages/web/src/javascripts/Constants/Strings.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Platform, SNApplication } from '@standardnotes/snjs'
import { escapeHtmlString, Platform, SNApplication } from '@standardnotes/snjs'
import { getPlatform, isDesktopApplication } from '../Utils'

/** @generic */
Expand Down Expand Up @@ -39,9 +39,10 @@ export const STRING_EDIT_LOCKED_ATTEMPT =
export const STRING_RESTORE_LOCKED_ATTEMPT =
"This note has editing disabled. If you'd like to restore it to a previous revision, enable editing and try again."
export function StringDeleteNote(title: string, permanently: boolean) {
const escapedTitle = escapeHtmlString(title)
return permanently
? `Are you sure you want to permanently delete ${title}?`
: `Are you sure you want to move ${title} to the trash?`
? `Are you sure you want to permanently delete ${escapedTitle}?`
: `Are you sure you want to move ${escapedTitle} to the trash?`
}
export function StringEmptyTrash(count: number) {
return `Are you sure you want to permanently delete ${count} note(s)?`
Expand Down Expand Up @@ -135,17 +136,19 @@ export const StringUtils = {
},
deleteNotes(permanently: boolean, notesCount = 1, title?: string): string {
if (notesCount === 1) {
const escapedTitle = escapeHtmlString(title || '')
return permanently
? `Are you sure you want to permanently delete ${title}?`
: `Are you sure you want to move ${title} to the trash?`
? `Are you sure you want to permanently delete ${escapedTitle}?`
: `Are you sure you want to move ${escapedTitle} to the trash?`
} else {
return permanently
? 'Are you sure you want to permanently delete these notes?'
: 'Are you sure you want to move these notes to the trash?'
}
},
deleteFile(title: string): string {
return `Are you sure you want to permanently delete ${title}?`
const escapedTitle = escapeHtmlString(title)
return `Are you sure you want to permanently delete ${escapedTitle}?`
},
archiveLockedNotesAttempt(archive: boolean, notesCount = 1): string {
const archiveString = archive ? 'archive' : 'unarchive'
Expand All @@ -158,4 +161,12 @@ export const StringUtils = {
? "This note has editing disabled. If you'd like to delete it, enable editing, and try again."
: "One or more of these notes have editing disabled. If you'd like to delete them, make sure editing is enabled on all of them, and try again."
},
deleteTag(title: string): string {
const escapedTitle = escapeHtmlString(title)
return `Delete tag "${escapedTitle}"?`
},
cannotUploadFile(name: string): string {
const escapedName = escapeHtmlString(name)
return `Cannot upload file "${escapedName}"`
},
}
4 changes: 2 additions & 2 deletions packages/web/src/javascripts/Controllers/FilesController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ export class FilesController extends AbstractViewController<FilesControllerEvent

deleteFile = async (file: FileItem) => {
const shouldDelete = await confirmDialog({
text: `Are you sure you want to permanently delete "${file.name}"?`,
text: StringUtils.deleteFile(file.name),
confirmButtonStyle: 'danger',
})
if (shouldDelete) {
Expand Down Expand Up @@ -440,7 +440,7 @@ export class FilesController extends AbstractViewController<FilesControllerEvent
`This file exceeds the limits supported in this browser. To upload files greater than ${
this.maxFileSize / BYTES_IN_ONE_MEGABYTE
}MB, please use the desktop application or the Chrome browser.`,
`Cannot upload file "${file.name}"`,
StringUtils.cannotUploadFile(file.name),
)
.catch(console.error)
return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
VaultDisplayService,
VaultDisplayServiceEvent,
} from '@standardnotes/ui-services'
import { STRING_DELETE_TAG } from '@/Constants/Strings'
import { STRING_DELETE_TAG, StringUtils } from '@/Constants/Strings'
import { SMART_TAGS_FEATURE_NAME } from '@/Constants/Constants'
import {
ContentType,
Expand Down Expand Up @@ -604,7 +604,7 @@ export class NavigationController
let shouldDelete = !userTriggered
if (userTriggered) {
shouldDelete = await confirmDialog({
title: `Delete tag "${tag.title}"?`,
title: StringUtils.deleteTag(tag.title),
text: STRING_DELETE_TAG,
confirmButtonStyle: 'danger',
})
Expand Down
Loading