From 8bb528e492db495d0c5c0743591a67effdbca2bb Mon Sep 17 00:00:00 2001 From: Safwan Samsudeen Date: Wed, 29 Jul 2026 12:20:53 +0530 Subject: [PATCH 1/3] fix(writer): keep list numbering continuous, plus assorted drive/writer fixes fix(writer): join adjacent same-type list nodes after each transaction so deleting a block between two lists (or Enter-lifting an item out) no longer restarts numbering; map boundaries through tr.mapping, require matching list types, and collect top-level boundaries since descendants() skips the doc node fix(drive): let an open dialog's own Escape handler run instead of clearing the selection in GridView/ListView (preventDefault was blocking Reka's dismiss) fix(writer): focus the comment editor without scrolling, on mount and when it flips into edit mode refactor(drive): use the Button component for UploadTracker controls and MoveDialog breadcrumbs; tidy ConfirmDialog markup and use a solid trash button refactor(writer): drop the watermark logic from PDF and print export style(writer): sentence-case the settings labels style(drive): add gap in the UserListSettings avatar row Co-Authored-By: Claude Opus 4.8 (1M context) --- .../apps/drive/components/ConfirmDialog.vue | 16 ++-- .../src/apps/drive/components/GridView.vue | 4 + .../src/apps/drive/components/ListView.vue | 4 + .../components/Settings/UserListSettings.vue | 2 +- .../apps/drive/components/UploadTracker.vue | 8 +- .../drive/ui/drive/components/MoveDialog.vue | 18 ++-- .../apps/writer/components/CommentEditor.vue | 14 +++- .../src/apps/writer/components/CoreEditor.vue | 2 + .../apps/writer/components/WriterSettings.vue | 82 ++++--------------- .../writer/extensions/join-adjacent-lists.js | 55 +++++++++++++ frontend/src/apps/writer/utils/download.js | 23 ------ frontend/src/apps/writer/utils/index.js | 20 ----- 12 files changed, 113 insertions(+), 135 deletions(-) create mode 100644 frontend/src/apps/writer/extensions/join-adjacent-lists.js diff --git a/frontend/src/apps/drive/components/ConfirmDialog.vue b/frontend/src/apps/drive/components/ConfirmDialog.vue index 2ee2489218..239fcbfbf3 100644 --- a/frontend/src/apps/drive/components/ConfirmDialog.vue +++ b/frontend/src/apps/drive/components/ConfirmDialog.vue @@ -1,13 +1,11 @@ @@ -61,7 +59,7 @@ const dialogData = computed(() => { button: { label: 'Move to Trash', theme: 'red', - variant: 'subtle', + variant: 'solid', }, onSuccess: () => { getTrash.setData( diff --git a/frontend/src/apps/drive/components/GridView.vue b/frontend/src/apps/drive/components/GridView.vue index f5d264bc67..91e6f08ac9 100644 --- a/frontend/src/apps/drive/components/GridView.vue +++ b/frontend/src/apps/drive/components/GridView.vue @@ -165,6 +165,10 @@ onKeyDown('Escape', (e) => { e.target.tagName === 'TEXTAREA' ) return + // A dialog is open — let its own Escape-to-close handler take this + // keystroke instead of eating it via preventDefault (which blocks Reka's + // dismissable-layer check for unhandled Escape). + if (document.querySelector('[role="dialog"]')) return selections.value = new Set() e.preventDefault() }) diff --git a/frontend/src/apps/drive/components/ListView.vue b/frontend/src/apps/drive/components/ListView.vue index 2eb1f1784f..b958373351 100644 --- a/frontend/src/apps/drive/components/ListView.vue +++ b/frontend/src/apps/drive/components/ListView.vue @@ -259,6 +259,10 @@ onKeyDown('Escape', (e) => { e.target.tagName === 'TEXTAREA' ) return + // A dialog is open — let its own Escape-to-close handler take this + // keystroke instead of eating it via preventDefault (which blocks Reka's + // dismissable-layer check for unhandled Escape). + if (document.querySelector('[role="dialog"]')) return container.value.selections.clear() e.preventDefault() }) diff --git a/frontend/src/apps/drive/components/Settings/UserListSettings.vue b/frontend/src/apps/drive/components/Settings/UserListSettings.vue index c320eb9282..d576a8f240 100644 --- a/frontend/src/apps/drive/components/Settings/UserListSettings.vue +++ b/frontend/src/apps/drive/components/Settings/UserListSettings.vue @@ -102,7 +102,7 @@ class="flex items-center justify-start pr-4 gap-x-3 py-2" > -
+
{{ user.full_name }} {{ user.email }}
diff --git a/frontend/src/apps/drive/components/UploadTracker.vue b/frontend/src/apps/drive/components/UploadTracker.vue index 8a5932d5e6..7e13efab1c 100644 --- a/frontend/src/apps/drive/components/UploadTracker.vue +++ b/frontend/src/apps/drive/components/UploadTracker.vue @@ -20,12 +20,8 @@ {{ uploadsFailed.length == 1 ? 'upload' : 'uploads' }} failed
- - +
diff --git a/frontend/src/apps/drive/ui/drive/components/MoveDialog.vue b/frontend/src/apps/drive/ui/drive/components/MoveDialog.vue index f6133e8b4f..aaf1373334 100644 --- a/frontend/src/apps/drive/ui/drive/components/MoveDialog.vue +++ b/frontend/src/apps/drive/ui/drive/components/MoveDialog.vue @@ -95,12 +95,18 @@ {{ '/' }} - +
diff --git a/frontend/src/apps/writer/components/CommentEditor.vue b/frontend/src/apps/writer/components/CommentEditor.vue index 05df32e078..05de4df1ad 100644 --- a/frontend/src/apps/writer/components/CommentEditor.vue +++ b/frontend/src/apps/writer/components/CommentEditor.vue @@ -5,7 +5,7 @@ !disabled && !isEmpty && $emit('submit', editor) " @keydown.esc.stop="$emit('cancel', editor)">
- + diff --git a/frontend/src/apps/drive/components/EntityDialogs.vue b/frontend/src/apps/drive/components/EntityDialogs.vue index 8305463255..d27aca1dd6 100644 --- a/frontend/src/apps/drive/components/EntityDialogs.vue +++ b/frontend/src/apps/drive/components/EntityDialogs.vue @@ -1,17 +1,6 @@ diff --git a/frontend/src/apps/drive/components/ErrorPage.vue b/frontend/src/apps/drive/components/ErrorPage.vue index f96118e60f..3966d483ba 100644 --- a/frontend/src/apps/drive/components/ErrorPage.vue +++ b/frontend/src/apps/drive/components/ErrorPage.vue @@ -33,7 +33,6 @@ diff --git a/frontend/src/apps/drive/components/GenericPage.vue b/frontend/src/apps/drive/components/GenericPage.vue index 12b307c08d..4e491b1b2e 100644 --- a/frontend/src/apps/drive/components/GenericPage.vue +++ b/frontend/src/apps/drive/components/GenericPage.vue @@ -4,15 +4,16 @@ -
+
- - + +
-
- {{ file.file_name }} -
+ +
+ {{ file.file_name }} +
+
import { getIconUrl, getThumbnailUrl } from '@/apps/drive/utils/files' import { ref, computed } from 'vue' +import InlineRenameInput from './InlineRenameInput.vue' const props = defineProps({ file: Object }) const { src, fallback } = getThumbnailUrl(props.file, 'grid') diff --git a/frontend/src/apps/drive/components/GridView.vue b/frontend/src/apps/drive/components/GridView.vue index 91e6f08ac9..65cd8e68f5 100644 --- a/frontend/src/apps/drive/components/GridView.vue +++ b/frontend/src/apps/drive/components/GridView.vue @@ -2,7 +2,8 @@
+
diff --git a/frontend/src/apps/drive/pages/Folder.vue b/frontend/src/apps/drive/pages/Folder.vue index c8fcb46fc9..f2c98e84d7 100644 --- a/frontend/src/apps/drive/pages/Folder.vue +++ b/frontend/src/apps/drive/pages/Folder.vue @@ -39,6 +39,7 @@ const getFolderContents = createResource({ }), cache: ['folder', props.entityName], }) +getFolderContents.paginated = true setCache(getFolderContents, ['folder', props.entityName]) const onSuccess = (entity) => { diff --git a/frontend/src/apps/drive/pages/Notifications.vue b/frontend/src/apps/drive/pages/Notifications.vue index d1463a3245..85120a1b36 100644 --- a/frontend/src/apps/drive/pages/Notifications.vue +++ b/frontend/src/apps/drive/pages/Notifications.vue @@ -1,26 +1,23 @@ diff --git a/frontend/src/apps/drive/ui/drive/index.js b/frontend/src/apps/drive/ui/drive/index.js index 2d2120e8e9..e57df851ec 100644 --- a/frontend/src/apps/drive/ui/drive/index.js +++ b/frontend/src/apps/drive/ui/drive/index.js @@ -1,5 +1,4 @@ export { default as ShareDialog } from './components/ShareDialog.vue' export { default as MoveDialog } from './components/MoveDialog.vue' export { default as InfoDialog } from './components/InfoDialog.vue' -export { default as RenameDialog } from './components/RenameDialog.vue' export { default as TeamSelector } from './components/TeamSelector.vue' diff --git a/frontend/src/apps/drive/utils/confirmActions.js b/frontend/src/apps/drive/utils/confirmActions.js new file mode 100644 index 0000000000..16b7db75d8 --- /dev/null +++ b/frontend/src/apps/drive/utils/confirmActions.js @@ -0,0 +1,112 @@ +import { dialog, call, toast } from 'frappe-ui' +import { useTimeAgo } from '@vueuse/core' +import { getTrash, toggleFav, clearRecent, clearTrash } from '@/apps/drive/resources/files.js' +import { sortEntities } from '@/apps/drive/utils/files.js' + +function itemString(entities) { + return entities.length === 1 ? 'an item' : `${entities.length} items` +} + +function entityLabel(entities) { + return entities.length > 1 ? 'These items' : `"${entities[0].file_name}"` +} + +function entityNames(entities) { + return JSON.stringify(entities.map((entity) => entity.name)) +} + +export function confirmRestore(entities, { onSuccess } = {}) { + const label = itemString(entities) + dialog.confirm({ + title: `Restore ${label}`, + message: `${entityLabel(entities)} will be restored to ${ + entities.length === 1 ? 'its original location' : 'their original locations' + }.`, + confirmLabel: 'Restore', + onConfirm: async () => { + await call('suite.drive.api.files.remove_or_restore', { + entity_names: entityNames(entities), + }) + const names = entities.map((entity) => entity.name) + getTrash.setData((d) => d.filter((k) => !names.includes(k.name))) + toast.success(`Restored ${label}.`) + onSuccess?.() + }, + }) +} + +export function confirmRemove(entities, { onSuccess } = {}) { + const label = itemString(entities) + dialog.confirm({ + title: `Move ${label} to Trash`, + message: `${entityLabel(entities)} will be moved to Trash. Items in trash are deleted forever after 30 days.`, + confirmLabel: 'Move to Trash', + theme: 'red', + onConfirm: async () => { + await call('suite.drive.api.files.remove_or_restore', { + entity_names: entityNames(entities), + }) + getTrash.setData( + sortEntities([ + ...getTrash.data, + ...entities.map((entity) => { + entity.modified = Date() + entity.relativeModified = useTimeAgo(entity.modified) + return entity + }), + ]), + ) + toast.success(`Moved ${label} to Trash.`) + onSuccess?.() + }, + }) +} + +export function confirmDeleteForever(entities, { onSuccess } = {}) { + const label = itemString(entities) + dialog.danger({ + title: `Delete ${label}`, + message: `${entityLabel(entities)} will be deleted — you can no longer access it. This is an irreversible action.`, + confirmLabel: 'Delete forever', + onConfirm: async () => { + await call('suite.drive.api.files.delete_entities', { + entity_names: entityNames(entities), + }) + toast.success(`Deleted ${label}.`) + onSuccess?.() + }, + }) +} + +export function confirmClearRecents() { + dialog.confirm({ + title: 'Are you sure?', + message: 'All your recently viewed files will be cleared.', + confirmLabel: 'Clear', + onConfirm: async () => { + await clearRecent.submit() + }, + }) +} + +export function confirmClearFavourites() { + dialog.confirm({ + title: 'Are you sure?', + message: 'All your favourite items will be cleared.', + confirmLabel: 'Clear', + onConfirm: async () => { + await toggleFav.submit() + }, + }) +} + +export function confirmClearTrash() { + dialog.danger({ + title: 'Clear your Trash', + message: 'All items in your Trash will be deleted forever. This is an irreversible process.', + confirmLabel: 'Delete', + onConfirm: async () => { + await clearTrash.submit() + }, + }) +} diff --git a/frontend/src/apps/drive/utils/download.js b/frontend/src/apps/drive/utils/download.js index 4603268029..efd84845f9 100644 --- a/frontend/src/apps/drive/utils/download.js +++ b/frontend/src/apps/drive/utils/download.js @@ -24,7 +24,7 @@ async function prepareArchive(entities) { try { ;({ token } = await call('suite.drive.api.files.download_folder', { entities: names })) } catch (error) { - toast({ title: error?.message || 'Download failed', type: 'error' }) + toast.error(error?.message || 'Download failed') return } diff --git a/frontend/src/apps/drive/utils/files.js b/frontend/src/apps/drive/utils/files.js index 31de942e07..6b40f42a5d 100644 --- a/frontend/src/apps/drive/utils/files.js +++ b/frontend/src/apps/drive/utils/files.js @@ -471,12 +471,18 @@ const copyToClipboard = (str) => { export async function updateURLSlug(file_name) { const route = router.currentRoute.value await nextTick() + // Only the folder/file pages carry a `:slug` segment. Guard against callers + // firing on other pages (e.g. rename from a list view), where appending a + // slug would produce a bogus path like `/drive/` that matches no route. + if (!['drive-Folder', 'drive-File'].includes(route.name)) return const slug = slugger(file_name) if (route.params.slug !== slug) { - // Hacky, but we only want to update the URL - triggering a reload breaks a lot + // Hacky, but we only want to update the URL - triggering a reload breaks a lot. + // Preserve the existing history state so vue-router's back/forward tracking + // isn't wiped. const base = window.location.pathname.split('/').slice(0, 4).join('/') const new_path = base + (base.endsWith('/') ? '' : '/') + slug - history.replaceState({}, null, new_path) + history.replaceState(history.state, '', new_path) } } @@ -500,20 +506,15 @@ export function getLink(entity, copy = true, withDomain = true) { (entity.content_docname || entity.name) } else { link = `${ - withDomain ? window.location.origin + '/drive' : '' - }/${getLinkStem(entity)}` + withDomain ? window.location.origin : '' + }/drive/${getLinkStem(entity)}` } if (!copy) return link try { copyToClipboard(link).then(() => toast('Copied to your clipboard.')) } catch (err) { if (err.name === 'NotAllowedError') { - toast({ - icon: 'alert-triangle', - iconClasses: 'text-ink-red-6', - title: 'Clipboard permission denied', - position: 'bottom-right', - }) + toast('Clipboard permission denied') } else { console.error('Failed to copy link:', err) } diff --git a/frontend/src/apps/drive/utils/useInlineRename.js b/frontend/src/apps/drive/utils/useInlineRename.js new file mode 100644 index 0000000000..05012ae704 --- /dev/null +++ b/frontend/src/apps/drive/utils/useInlineRename.js @@ -0,0 +1,82 @@ +import { nextTick, ref } from 'vue' +import { renamingEntity, stopRename } from '@/apps/drive/data/selection' +import { rename } from '@/apps/drive/ui/drive/js/resources' + +// Types whose name is edited whole — they carry no user-facing extension. +const KEEP_WHOLE_TYPES = ['Document', 'Markdown', 'Link'] + +// Length of the base name (everything before the extension). Used to pre-select +// just the name on focus, leaving the extension visible but untouched — the +// behaviour of Finder/Explorer inline rename. +function baseNameLength(entity) { + const name = entity.file_name || '' + if (entity.is_folder || KEEP_WHOLE_TYPES.includes(entity.file_type)) { + return name.length + } + const dot = name.lastIndexOf('.') + return dot <= 0 ? name.length : dot +} + +// Inline rename for a single entity (a list row, grid tile, or breadcrumb). +// `source` is the entity object or a getter returning it (breadcrumb entities +// can load asynchronously). The entity is mutated optimistically so every view +// bound to it updates. +const now = () => + typeof performance !== 'undefined' && performance.now ? performance.now() : Date.now() + +export function useInlineRename(source) { + const entity = () => (typeof source === 'function' ? source() : source) + const draft = ref('') + const input = ref(null) + let openedAt = 0 + + function focusAndSelect(e) { + const el = input.value + if (!el || renamingEntity.value !== e.name) return + if (document.activeElement !== el) el.focus() + el.setSelectionRange(0, baseNameLength(e)) + } + + function start() { + const e = entity() + if (!e) return + draft.value = e.file_name || '' + openedAt = now() + // Focus + select once the input is in the DOM. Re-assert on a macrotask and + // a frame in case something reclaims focus a tick later. + nextTick(() => focusAndSelect(e)) + if (typeof setTimeout === 'function') setTimeout(() => focusAndSelect(e), 0) + if (typeof requestAnimationFrame === 'function') { + requestAnimationFrame(() => focusAndSelect(e)) + } + } + + // Blur handler: a breadcrumb rename is triggered by clicking the crumb, whose + // button is then removed from the DOM — that briefly drops focus to + // (blur with no relatedTarget) right after we focus the input. Committing on + // that transient blur would exit edit mode instantly. So within a short grace + // window, an aimless focus loss reclaims focus instead of committing; a real + // click-away (focus moved to another element, or after the field settles) + // still commits. + function blur(event) { + const e = entity() + if (!e || renamingEntity.value !== e.name) return + if (!event?.relatedTarget && now() - openedAt < 300) { + focusAndSelect(e) + return + } + submit() + } + + function submit() { + const e = entity() + if (!e || renamingEntity.value !== e.name) return + const title = draft.value.trim() + stopRename() + if (!title || title === e.file_name) return + e.file_name = title + rename.submit({ entity_name: e.name, new_title: title }) + } + + return { draft, input, start, submit, blur, cancel: stopRename } +} diff --git a/frontend/src/apps/writer/components/Dialogs.vue b/frontend/src/apps/writer/components/Dialogs.vue index 23e4524286..8f67134479 100644 --- a/frontend/src/apps/writer/components/Dialogs.vue +++ b/frontend/src/apps/writer/components/Dialogs.vue @@ -1,7 +1,6 @@ diff --git a/frontend/src/apps/drive/data/folderTree.js b/frontend/src/apps/drive/data/folderTree.js new file mode 100644 index 0000000000..c06333eca9 --- /dev/null +++ b/frontend/src/apps/drive/data/folderTree.js @@ -0,0 +1,114 @@ +import { computed, reactive, ref } from 'vue' +import { request } from 'frappe-ui' +import { prettyData, sortEntities } from '@/apps/drive/utils/files' + +export const expandedFolders = ref(new Set()) +export const folderChildren = reactive({}) + +// Skeleton rows shown while a subtree loads, so a huge folder doesn't flood the DOM. +export const MAX_SKELETONS = 20 + +export const loadedChildRows = computed(() => + Object.values(folderChildren).flatMap((node) => node.rows) +) + +// Guards against an earlier fetch of the same folder resolving after a later one. +const tokens = {} + +async function fetchChildren(name, sortOrder) { + const node = folderChildren[name] + const token = (tokens[name] = (tokens[name] || 0) + 1) + node.loading = true + let rows = [] + try { + const data = await request({ + url: '/api/method/suite.drive.api.list.files', + method: 'GET', + params: { + team: '', + entity_name: name, + order_by: sortOrder?.field || 'modified', + ascending: sortOrder?.ascending ?? false, + }, + credentials: 'include', + }) + const list = Array.isArray(data) ? data : (data?.message ?? []) + rows = sortEntities( + prettyData(list.filter((k) => !k.file_name?.startsWith('.'))), + sortOrder + ) + } catch { + rows = [] + } + if (tokens[name] !== token || !folderChildren[name]) return + folderChildren[name].rows = rows + folderChildren[name].loading = false +} + +function descendants(name, acc = []) { + for (const row of folderChildren[name]?.rows ?? []) { + acc.push(row.name) + if (folderChildren[row.name]) descendants(row.name, acc) + } + return acc +} + +// Returns the names dropped from view, so the caller can deselect them. +export function toggleFolder(row, sortOrder) { + const next = new Set(expandedFolders.value) + let collapsed = [] + if (next.has(row.name)) { + collapsed = descendants(row.name) + for (const name of [row.name, ...collapsed]) { + next.delete(name) + delete folderChildren[name] + } + } else { + next.add(row.name) + if (!folderChildren[row.name]) { + folderChildren[row.name] = { rows: [], loading: true } + fetchChildren(row.name, sortOrder) + } + } + expandedFolders.value = next + return collapsed +} + +export function refreshFolder(name, sortOrder) { + if (folderChildren[name]) fetchChildren(name, sortOrder) +} + +export function refreshExpanded(sortOrder) { + Object.keys(folderChildren).forEach((name) => fetchChildren(name, sortOrder)) +} + +export function removeFromTree(names) { + Object.values(folderChildren).forEach((node) => { + node.rows = node.rows.filter(({ name }) => !names.includes(name)) + }) +} + +export function resetTree() { + expandedFolders.value = new Set() + Object.keys(folderChildren).forEach((name) => delete folderChildren[name]) +} + +// Rows of an expanded folder render inline under it, so a flat list of rows +// becomes a depth-tagged sequence. Keys are path-based: the same entity can +// legitimately appear at two depths (e.g. a folder and its child both matching +// in Favourites). +export function flattenRows(rows, depth = 0, parentKey = '', out = []) { + for (const row of rows) { + const key = parentKey ? `${parentKey}>${row.name}` : row.name + out.push({ row, depth, key }) + if (!row.is_folder || !expandedFolders.value.has(row.name)) continue + const node = folderChildren[row.name] + if (node?.loading) + for (let i = 0; i < Math.min(row.child_count || 1, MAX_SKELETONS); i++) + out.push({ placeholder: 'loading', depth: depth + 1, key: `${key}:loading:${i}` }) + else if (!node?.rows.length) + out.push({ placeholder: 'empty', depth: depth + 1, key: `${key}:empty` }) + else flattenRows(node.rows, depth + 1, key, out) + } + return out +} diff --git a/frontend/src/apps/drive/data/folderTree.test.ts b/frontend/src/apps/drive/data/folderTree.test.ts new file mode 100644 index 0000000000..ced4e5c4c0 --- /dev/null +++ b/frontend/src/apps/drive/data/folderTree.test.ts @@ -0,0 +1,244 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ request: vi.fn() })) + +vi.mock('frappe-ui', () => ({ request: mocks.request })) +vi.mock('@/apps/drive/utils/files', () => ({ + prettyData: (rows: unknown[]) => rows, + sortEntities: (rows: { file_name?: string }[], order?: { ascending?: boolean }) => + [...rows].sort((a, b) => + (order?.ascending === false ? -1 : 1) * + (a.file_name ?? '').localeCompare(b.file_name ?? '') + ), +})) + +import { + MAX_SKELETONS, + expandedFolders, + flattenRows, + folderChildren, + loadedChildRows, + refreshFolder, + removeFromTree, + resetTree, + toggleFolder, +} from './folderTree' + +type Row = { + name: string + file_name?: string + is_folder?: boolean + child_count?: number +} + +const folder = (name: string, child_count = 3): Row => ({ + name, + file_name: name, + is_folder: true, + child_count, +}) +const file = (name: string): Row => ({ name, file_name: name }) + +const flush = () => new Promise((resolve) => setTimeout(resolve, 0)) + +beforeEach(() => { + resetTree() + mocks.request.mockReset() +}) + +describe('toggleFolder', () => { + it('expands, fetches children and exposes them as loaded rows', async () => { + mocks.request.mockResolvedValue([file('b'), file('a')]) + + toggleFolder(folder('parent'), { field: 'file_name', ascending: true }) + expect(expandedFolders.value.has('parent')).toBe(true) + expect(folderChildren.parent.loading).toBe(true) + + await flush() + expect(folderChildren.parent.loading).toBe(false) + expect(folderChildren.parent.rows.map((r) => r.name)).toEqual(['a', 'b']) + expect(loadedChildRows.value.map((r) => r.name)).toEqual(['a', 'b']) + + const [{ params }] = mocks.request.mock.calls[0] + expect(params).toMatchObject({ + entity_name: 'parent', + order_by: 'file_name', + ascending: true, + }) + }) + + it('unwraps a message-envelope response and drops dotfiles', async () => { + mocks.request.mockResolvedValue({ message: [file('.hidden'), file('shown')] }) + + toggleFolder(folder('parent')) + await flush() + + expect(folderChildren.parent.rows.map((r) => r.name)).toEqual(['shown']) + }) + + it('leaves an empty subtree behind when the fetch fails', async () => { + mocks.request.mockRejectedValue(new Error('403')) + + toggleFolder(folder('parent')) + await flush() + + expect(folderChildren.parent).toMatchObject({ rows: [], loading: false }) + }) + + it('does not refetch a subtree that is already cached', async () => { + mocks.request.mockResolvedValue([file('a')]) + toggleFolder(folder('parent')) + await flush() + + toggleFolder(folder('parent')) // collapse + toggleFolder(folder('parent')) // expand again + await flush() + // The collapse drops the cache, so this is a fresh fetch — but only one. + expect(mocks.request).toHaveBeenCalledTimes(2) + }) + + it('collapsing drops the whole subtree and reports the hidden names', async () => { + mocks.request.mockResolvedValueOnce([file('child'), folder('nested')]) + toggleFolder(folder('parent')) + await flush() + + mocks.request.mockResolvedValueOnce([file('deep')]) + toggleFolder(folder('nested')) + await flush() + + const collapsed = toggleFolder(folder('parent')) + + expect(collapsed.sort()).toEqual(['child', 'deep', 'nested']) + expect(expandedFolders.value.size).toBe(0) + expect(folderChildren.parent).toBeUndefined() + expect(folderChildren.nested).toBeUndefined() + expect(loadedChildRows.value).toEqual([]) + }) + + it('ignores a stale response that resolves after a newer one', async () => { + let resolveFirst: (rows: Row[]) => void = () => {} + mocks.request.mockReturnValueOnce( + new Promise((resolve) => (resolveFirst = resolve)) + ) + toggleFolder(folder('parent')) + + mocks.request.mockResolvedValueOnce([file('fresh')]) + refreshFolder('parent') + await flush() + expect(folderChildren.parent.rows.map((r) => r.name)).toEqual(['fresh']) + + resolveFirst([file('stale')]) + await flush() + expect(folderChildren.parent.rows.map((r) => r.name)).toEqual(['fresh']) + }) +}) + +describe('removeFromTree / resetTree', () => { + it('removes moved or deleted rows from every loaded subtree', async () => { + mocks.request.mockResolvedValue([file('a'), file('b')]) + toggleFolder(folder('parent')) + await flush() + + removeFromTree(['a']) + expect(folderChildren.parent.rows.map((r) => r.name)).toEqual(['b']) + }) + + it('refreshFolder is a no-op for a folder that was never expanded', () => { + refreshFolder('unknown') + expect(mocks.request).not.toHaveBeenCalled() + }) + + it('resetTree clears expansion and cached children', async () => { + mocks.request.mockResolvedValue([file('a')]) + toggleFolder(folder('parent')) + await flush() + + resetTree() + expect(expandedFolders.value.size).toBe(0) + expect(loadedChildRows.value).toEqual([]) + }) +}) + +describe('flattenRows', () => { + it('leaves a collapsed list untouched at depth 0', () => { + const rows = [folder('parent'), file('loose')] + expect(flattenRows(rows)).toEqual([ + { row: rows[0], depth: 0, key: 'parent' }, + { row: rows[1], depth: 0, key: 'loose' }, + ]) + }) + + it('inlines children under their folder with an incremented depth', async () => { + mocks.request.mockResolvedValueOnce([file('child'), folder('nested')]) + toggleFolder(folder('parent')) + await flush() + mocks.request.mockResolvedValueOnce([file('deep')]) + toggleFolder(folder('nested')) + await flush() + + const items = flattenRows([folder('parent'), file('loose')]) + + expect(items.map((i) => [i.row?.name ?? i.placeholder, i.depth])).toEqual([ + ['parent', 0], + ['child', 1], + ['nested', 1], + ['deep', 2], + ['loose', 0], + ]) + }) + + it('keys rows by path so the same entity can appear at two depths', async () => { + mocks.request.mockResolvedValue([file('dupe')]) + toggleFolder(folder('parent')) + await flush() + + const items = flattenRows([folder('parent'), file('dupe')]) + const keys = items.map((i) => i.key) + + expect(keys).toEqual(['parent', 'parent>dupe', 'dupe']) + expect(new Set(keys).size).toBe(keys.length) + }) + + it('shows one skeleton per known child while loading, capped', () => { + mocks.request.mockReturnValue(new Promise(() => {})) + + toggleFolder(folder('small', 2)) + let items = flattenRows([folder('small', 2)]) + expect(items.filter((i) => i.placeholder === 'loading')).toHaveLength(2) + expect(items.every((i) => !i.row || i.depth === 0)).toBe(true) + + toggleFolder(folder('huge', 500)) + items = flattenRows([folder('huge', 500)]) + expect(items.filter((i) => i.placeholder === 'loading')).toHaveLength( + MAX_SKELETONS + ) + }) + + it('falls back to a single skeleton when the child count is unknown', () => { + mocks.request.mockReturnValue(new Promise(() => {})) + const unknown = { name: 'x', file_name: 'x', is_folder: true } + + toggleFolder(unknown) + const items = flattenRows([unknown]) + + expect(items.filter((i) => i.placeholder === 'loading')).toHaveLength(1) + }) + + it('marks an expanded folder that turned out to be empty', async () => { + mocks.request.mockResolvedValue([]) + toggleFolder(folder('parent')) + await flush() + + const items = flattenRows([folder('parent')]) + expect(items[1]).toMatchObject({ placeholder: 'empty', depth: 1 }) + }) + + it('does not expand a non-folder row that shares a name with an expanded folder', async () => { + mocks.request.mockResolvedValue([file('a')]) + toggleFolder(folder('parent')) + await flush() + + const items = flattenRows([{ name: 'parent', file_name: 'parent' }]) + expect(items).toHaveLength(1) + }) +}) diff --git a/frontend/src/apps/drive/index.css b/frontend/src/apps/drive/index.css index 7362935bac..306d6065f0 100644 --- a/frontend/src/apps/drive/index.css +++ b/frontend/src/apps/drive/index.css @@ -37,6 +37,19 @@ html.theme-switching * { background: #ebeef0; } +[data-theme='dark'] *::-webkit-scrollbar-thumb { + background: #4a4a4a; +} + +[data-theme='dark'] *::-webkit-scrollbar-track, +[data-theme='dark'] *::-webkit-scrollbar-corner { + background: transparent; +} + +[data-theme='dark'] * { + scrollbar-color: #4a4a4a transparent; +} + *::-webkit-scrollbar { width: 6px; height: 6px; diff --git a/frontend/src/apps/drive/resources/files.js b/frontend/src/apps/drive/resources/files.js index 10240ae0c8..f84454c2cc 100644 --- a/frontend/src/apps/drive/resources/files.js +++ b/frontend/src/apps/drive/resources/files.js @@ -113,7 +113,6 @@ export const getSlides = createResource({ transform(data) { data = data.map((k) => ({ ...k, - // Presentations carry `title`; the list/grid views key off `file_name`. file_name: k.title, content_doctype: PRESENTATION_CONTENT_DOCTYPE, mime_type: 'frappe/slides', diff --git a/frontend/src/apps/drive/ui/drive/js/resources.js b/frontend/src/apps/drive/ui/drive/js/resources.js index a05cdebfa4..6221b7a332 100644 --- a/frontend/src/apps/drive/ui/drive/js/resources.js +++ b/frontend/src/apps/drive/ui/drive/js/resources.js @@ -57,6 +57,6 @@ export const rename = createResource({ } }, onError(error) { - toast.error(error) + toast.error(error.messages?.at(-1) || 'Could not rename this file.') }, }) diff --git a/frontend/src/apps/drive/utils/files.js b/frontend/src/apps/drive/utils/files.js index 6b40f42a5d..02ed6d0938 100644 --- a/frontend/src/apps/drive/utils/files.js +++ b/frontend/src/apps/drive/utils/files.js @@ -254,10 +254,22 @@ export const sortEntities = (rows, order) => { // Mutates directly const field = order.field const asc = order.ascending ? 1 : -1 + // Nullish sorts last either way — `>` is false in both directions against it + const compare = (x, y) => { + if (x == null || y == null) return x == y ? 0 : x == null ? 1 : -1 + return x === y ? 0 : x > y ? 1 : -1 + } rows.sort((a, b) => { - return a[field] == b[field] ? 0 : a[field] > b[field] ? asc : -asc + // Folders have no byte size, so they sink below files and sort by count + if (field === 'file_size' && a.is_folder !== b.is_folder) + return a.is_folder ? 1 : -1 + const sortField = + field === 'file_size' && a.is_folder ? 'child_count' : field + const primary = compare(a[sortField], b[sortField]) + if (primary) return primary * asc + return compare(a.file_name, b.file_name) * asc }) - if (order.smart) { + if (order.smart && field === 'file_name') { rows.sort((a, b) => { const [endA, endB] = trimCommonPrefix(a.file_name, b.file_name) if (!endA) return 0 diff --git a/frontend/src/apps/drive/utils/useInlineRename.js b/frontend/src/apps/drive/utils/useInlineRename.js index 05012ae704..0ed4db60ae 100644 --- a/frontend/src/apps/drive/utils/useInlineRename.js +++ b/frontend/src/apps/drive/utils/useInlineRename.js @@ -24,12 +24,24 @@ function baseNameLength(entity) { const now = () => typeof performance !== 'undefined' && performance.now ? performance.now() : Date.now() +// Window after opening during which any focus loss is treated as a transient +// steal to be reclaimed rather than a commit. Comfortably covers the tick or two +// it takes the trigger (a removed breadcrumb button, a closing menu) to settle. +const OPEN_GRACE_MS = 600 + export function useInlineRename(source) { const entity = () => (typeof source === 'function' ? source() : source) const draft = ref('') const input = ref(null) let openedAt = 0 + function selectBaseName() { + const e = entity() + const el = input.value + if (!e || !el) return + el.setSelectionRange(0, baseNameLength(e)) + } + function focusAndSelect(e) { const el = input.value if (!el || renamingEntity.value !== e.name) return @@ -51,18 +63,19 @@ export function useInlineRename(source) { } } - // Blur handler: a breadcrumb rename is triggered by clicking the crumb, whose - // button is then removed from the DOM — that briefly drops focus to - // (blur with no relatedTarget) right after we focus the input. Committing on - // that transient blur would exit edit mode instantly. So within a short grace - // window, an aimless focus loss reclaims focus instead of committing; a real - // click-away (focus moved to another element, or after the field settles) + // A breadcrumb rename is triggered by clicking the crumb, whose button is then + // removed from the DOM — that drops focus (to , or wherever the app's + // focus management sends it) right after we focus the input. Committing on + // that transient blur would exit edit mode instantly ("selects then instantly + // deselects"). So within the grace window right after opening, ANY focus loss + // reclaims focus instead of committing; after it settles, a real click-away // still commits. - function blur(event) { + function blur() { const e = entity() if (!e || renamingEntity.value !== e.name) return - if (!event?.relatedTarget && now() - openedAt < 300) { - focusAndSelect(e) + if (now() - openedAt < OPEN_GRACE_MS) { + // Reclaim on the next tick so the browser finishes moving focus first. + nextTick(() => focusAndSelect(e)) return } submit() @@ -74,9 +87,13 @@ export function useInlineRename(source) { const title = draft.value.trim() stopRename() if (!title || title === e.file_name) return + const previous = e.file_name e.file_name = title - rename.submit({ entity_name: e.name, new_title: title }) + rename.submit( + { entity_name: e.name, new_title: title }, + { onError: () => (e.file_name = previous) } + ) } - return { draft, input, start, submit, blur, cancel: stopRename } + return { draft, input, start, submit, blur, selectBaseName, cancel: stopRename } } diff --git a/frontend/src/apps/writer/components/ToC.vue b/frontend/src/apps/writer/components/ToC.vue index 39f4463f35..b404a8084a 100644 --- a/frontend/src/apps/writer/components/ToC.vue +++ b/frontend/src/apps/writer/components/ToC.vue @@ -5,9 +5,9 @@
-
+
- Table of Contents + Table of Contents
diff --git a/frontend/src/apps/writer/components/ToCMobile.vue b/frontend/src/apps/writer/components/ToCMobile.vue index 74ae84ba40..0a63257004 100644 --- a/frontend/src/apps/writer/components/ToCMobile.vue +++ b/frontend/src/apps/writer/components/ToCMobile.vue @@ -14,21 +14,15 @@
- - diff --git a/frontend/tailwind.config.js b/frontend/tailwind.config.js index c3c8b2e60c..a9ed179168 100644 --- a/frontend/tailwind.config.js +++ b/frontend/tailwind.config.js @@ -1,4 +1,4 @@ -import frappeUIPreset from 'frappe-ui/tailwind' +import frappeUIPreset from "frappe-ui/tailwind"; /** @type {import('tailwindcss').Config} */ export default { @@ -18,7 +18,7 @@ export default { ], variants: { extend: { - display: ['group-hover'], + display: ["group-hover"], }, }, -} +}; diff --git a/suite/drive/api/list.py b/suite/drive/api/list.py index b062741551..f7d7d9a405 100644 --- a/suite/drive/api/list.py +++ b/suite/drive/api/list.py @@ -91,12 +91,9 @@ def _get_children_count(files): def _get_slide_counts(rows): """ - Returns a dict mapping presentation docnames to their slide count, for the - presentation rows among `rows`. Drive stores these as files pointing at a - Presentation doc, so the count has to come from the Slides side. + Returns a dict mapping presentation docnames to their slide count. """ - # Imported lazily: suite.slides imports from suite.drive, so a module-level - # import here would be circular. + # Imported here as suite.slides imports from suite.drive. from suite.slides.doctype.presentation.presentation import get_slide_counts return get_slide_counts( diff --git a/suite/slides/doctype/presentation/presentation.py b/suite/slides/doctype/presentation/presentation.py index 1d393cac06..33295e6f55 100644 --- a/suite/slides/doctype/presentation/presentation.py +++ b/suite/slides/doctype/presentation/presentation.py @@ -216,7 +216,7 @@ def get_presentations() -> list[dict]: def get_slide_counts(presentation_names: list[str]) -> dict[str, int]: - """Slide count per presentation, as one grouped query rather than a COUNT each.""" + """Returns a dict mapping presentation names to their slide count.""" if not presentation_names: return {}