feat: update reset thumbnail feature #306
Conversation
|
centralize thumbnail ext logic and use site config API |
| // Ensure supported thumbnail extensions are primed when menu opens | ||
| useEffect(() => { | ||
| if (contextMenuOpen) { | ||
| dispatch(primeThumbExtsCache()); |
There was a problem hiding this comment.
Wait... does this mean adding an extra request for whenever a new context menu is open for the first time?
I don't think it is worth the cost since this new feature is not commonly used. We need to revisit the design to avoid introducing new requests.
Two options:
- Just ignore the supported thumbnail extension, show this option for all file with thumbnail disabled. I understand it will allow this context menu option visable for files does not support thumbnails, but that's OK.
- Still ignore the supported thumbnail extension and show the option. But when user actually clicked the option, request supported extensions from backend and check, popup an error/warning for files that are not supported.
There was a problem hiding this comment.
I think the second option is better, i will modify it
There was a problem hiding this comment.
This is not resolved. The extra request is still sent every time a user opens the explorer page.
|
drop reset API; use PATCH metadata and reload only selected thumbnails |
| @@ -292,6 +292,24 @@ export function getFileThumb(path: string, contextHint?: string): ThunkResponse< | |||
| }; | |||
| } | |||
|
|
|||
| // Thin wrapper to query supported thumbnail extensions from backend | |||
| export function getThumbExts(): ThunkResponse<{ thumb_exts?: string[] }> { | |||
There was a problem hiding this comment.
Please reuse
frontend/src/redux/thunks/site.ts
Line 6 in 3596160
| // Ensure supported thumbnail extensions are primed when menu opens | ||
| useEffect(() => { | ||
| if (contextMenuOpen) { | ||
| dispatch(primeThumbExtsCache()); |
There was a problem hiding this comment.
This is not resolved. The extra request is still sent every time a user opens the explorer page.
| cache instanceof Set | ||
| ? targets.some((f) => f.type == FileType.file && cache.has((fileExtension(f.name) || "").toLowerCase())) | ||
| : false; | ||
| display.showResetThumb = display.hasFile && anySupported; |
There was a problem hiding this comment.
Load thumb exts only after user clicked the context menu option.
Here you just need something like
display.showResetThumb = display.hasFile && has update permission && has thumb disabled key;
| import { getThumbExts } from "../../api/api.ts"; | ||
|
|
||
| // --- Cached supported thumbnail extensions helpers --- | ||
| let __thumbExtsCache: Set<string> | null | undefined = undefined; // undefined: not fetched, null: unknown/fallback |
There was a problem hiding this comment.
This whole file does not need to exist.
For cache layer for thumb exts, you can reuse
frontend/src/redux/siteConfigSlice.ts
Line 27 in 3596160
It has a cache status field
loaded
isThumbExtSupportedSync is redundant. Use
Line 87 in 3596160
xxx.indexOf is sufficent.
| tryLoadThumbSrc(); | ||
| }, [inView]); | ||
| }, [inView, file, file.metadata?.[Metadata.thumbDisabled]]); |
There was a problem hiding this comment.
This will trigger a reload of the thumbnail after reset. But you already loaded once in https://github.com/cloudreve/frontend/pull/306/files#diff-acc4829d8b9ac4ca5b1f91063828fb9acd9704b8d210258944946090996786e1R1201
So two identical requests will be made?
| .map((f) => fm.list?.files.find((ff) => ff.path === f.path) || f) | ||
| .filter((f): f is FileResponse => !!f); | ||
| // 并发触发 GET /file/thumb | ||
| await Promise.allSettled(toPrefetch.map((f) => dispatch(loadFileThumb(FileManagerIndex.main, f)))); |
There was a problem hiding this comment.
/file/thumb is expensive. Consider not loading it proactively. Just reset the thumbnail status, and replying on user scroll in view to trigger the loading.
feat: update reset thumbnail feature
Implementation of Issue cloudreve/cloudreve#2373