From 9a079d5d4c66a64db5d5bec47cbffd5328dffeef Mon Sep 17 00:00:00 2001 From: Younn Olivier Date: Mon, 31 Oct 2022 08:20:31 +0000 Subject: [PATCH] [FIX] web_editor, web_unsplash: fix debounced search from media dialog Following this flow on the Media Dialog: - Type "hell" - Wait a few ms so that the RPC to search images starts - Type "o" - The RPC of before finishes, the o is removed and no new search is done Before this commit, the search input value of the SearchMedia component was coming from a "needle" props. This needle prop was a state on the parent component (FileSelector), set through a debounced handler. This is not a good design: - Type "hell": after 1000ms, the debounced handler will be executed with the value "hell" - While the handler is executing: type "o" - When the handler finishes, it has set the needle state value to "hell" on the parent. This will rerender the SearchMedia with "hell" as a needle prop, and the "hello" input will be replaced with "hell". Instead of doing that, the SearchMedia component should have its own input state, which models the input element. On that state, we use an effect to call the debounced "search" callback. This way, the SearchMedia is less dependent on its parent: it only rerenders when its input element changes. Additionally, the fetch results are only used if they are the ones from the last call to `search`. Dedicated `KeepLast` instances are used for attachments, media library and unsplash. task-3060679 closes odoo/odoo#113188 X-original-commit: 08859ca7939454303e34dc39c831ac8f75773c3d Signed-off-by: Romain Derie (rde) Co-authored-by: Arthur Detroux Co-authored-by: Benoit Socias --- .../components/media_dialog/file_selector.js | 16 +++++++++++--- .../components/media_dialog/image_selector.js | 22 ++++++++++++++----- .../components/media_dialog/search_media.js | 22 ++++++++++++++----- .../components/media_dialog/image_selector.js | 18 ++++++++++----- 4 files changed, 58 insertions(+), 20 deletions(-) diff --git a/addons/web_editor/static/src/components/media_dialog/file_selector.js b/addons/web_editor/static/src/components/media_dialog/file_selector.js index 07cdc599570..61e8bdafd49 100644 --- a/addons/web_editor/static/src/components/media_dialog/file_selector.js +++ b/addons/web_editor/static/src/components/media_dialog/file_selector.js @@ -3,6 +3,7 @@ import { useService } from '@web/core/utils/hooks'; import { ConfirmationDialog } from '@web/core/confirmation_dialog/confirmation_dialog'; import { Dialog } from '@web/core/dialog/dialog'; +import { KeepLast } from "@web/core/utils/concurrency"; import { SearchMedia } from './search_media'; import { Component, xml, useState, useRef, onWillStart } from "@odoo/owl"; @@ -136,6 +137,7 @@ export class FileSelector extends Component { setup() { this.orm = useService('orm'); this.uploadService = useService('upload'); + this.keepLast = new KeepLast(); this.state = useState({ attachments: [], @@ -216,13 +218,21 @@ export class FileSelector extends Component { } async loadMore() { - const newAttachments = await this.fetchAttachments(this.NUMBER_OF_ATTACHMENTS_TO_DISPLAY, this.state.attachments.length); - this.state.attachments.push(...newAttachments); + return this.keepLast.add(this.fetchAttachments(this.NUMBER_OF_ATTACHMENTS_TO_DISPLAY, this.state.attachments.length)).then((newAttachments) => { + // This is never reached if another search or loadMore occurred. + this.state.attachments.push(...newAttachments); + }); } async search(needle) { + // Prepare in case loadMore results are obtained instead. + this.state.attachments = []; + // Fetch attachments relies on the state's needle. this.state.needle = needle; - this.state.attachments = await this.fetchAttachments(this.NUMBER_OF_ATTACHMENTS_TO_DISPLAY, 0); + return this.keepLast.add(this.fetchAttachments(this.NUMBER_OF_ATTACHMENTS_TO_DISPLAY, 0)).then((attachments) => { + // This is never reached if a new search occurred. + this.state.attachments = attachments; + }); } async uploadFiles(files) { diff --git a/addons/web_editor/static/src/components/media_dialog/image_selector.js b/addons/web_editor/static/src/components/media_dialog/image_selector.js index 426853e7468..4578f349959 100644 --- a/addons/web_editor/static/src/components/media_dialog/image_selector.js +++ b/addons/web_editor/static/src/components/media_dialog/image_selector.js @@ -3,6 +3,7 @@ import { useService } from '@web/core/utils/hooks'; import { getCSSVariableValue, DEFAULT_PALETTE } from 'web_editor.utils'; import { Attachment, FileSelector, IMAGE_MIMETYPES, IMAGE_EXTENSIONS } from './file_selector'; +import { KeepLast } from "@web/core/utils/concurrency"; import { useRef, useState, useEffect } from "@odoo/owl"; @@ -24,6 +25,10 @@ export class AutoResizeImage extends Attachment { } async onImageLoaded() { + if (!this.image.el) { + // Do not fail if already removed. + return; + } if (this.props.onLoaded) { await this.props.onLoaded(this.image.el); } @@ -41,6 +46,7 @@ export class ImageSelector extends FileSelector { super.setup(); this.rpc = useService('rpc'); + this.keepLastLibraryMedia = new KeepLast(); this.state.libraryMedia = []; this.state.libraryResults = null; @@ -177,8 +183,10 @@ export class ImageSelector extends FileSelector { if (!this.props.useMediaLibrary) { return; } - const { media } = await this.fetchLibraryMedia(this.state.libraryMedia.length); - this.state.libraryMedia.push(...media); + return this.keepLastLibraryMedia.add(this.fetchLibraryMedia(this.state.libraryMedia.length)).then(({ media }) => { + // This is never reached if another search or loadMore occurred. + this.state.libraryMedia.push(...media); + }); } async search(...args) { @@ -189,9 +197,13 @@ export class ImageSelector extends FileSelector { if (!this.state.needle) { this.state.searchService = 'all'; } - const { media, results } = await this.fetchLibraryMedia(0); - this.state.libraryMedia = media; - this.state.libraryResults = results; + this.state.libraryMedia = []; + this.state.libraryResults = 0; + return this.keepLastLibraryMedia.add(this.fetchLibraryMedia(0)).then(({ media, results }) => { + // This is never reached if a new search occurred. + this.state.libraryMedia = media; + this.state.libraryResults = results; + }); } async onClickAttachment(attachment) { diff --git a/addons/web_editor/static/src/components/media_dialog/search_media.js b/addons/web_editor/static/src/components/media_dialog/search_media.js index 50e3ae88457..6674baa63e2 100644 --- a/addons/web_editor/static/src/components/media_dialog/search_media.js +++ b/addons/web_editor/static/src/components/media_dialog/search_media.js @@ -1,21 +1,31 @@ /** @odoo-module **/ -import { debounce } from '@web/core/utils/timing'; +import { useDebounced } from '@web/core/utils/timing'; import { useAutofocus } from '@web/core/utils/hooks'; -import { Component, onWillUnmount, xml } from "@odoo/owl"; +import { Component, xml, useEffect, useState } from "@odoo/owl"; export class SearchMedia extends Component { setup() { useAutofocus(); - this.search = debounce((ev) => this.props.search(ev.target.value), 1000); - onWillUnmount(() => { - this.search.cancel(); + this.debouncedSearch = useDebounced(this.props.search, 1000); + + this.state = useState({ + input: this.props.needle || '', }); + + useEffect((input) => { + // Do not trigger a search on the initial render. + if (this.hasRendered) { + this.debouncedSearch(input); + } else { + this.hasRendered = true; + } + }, () => [this.state.input]); } } SearchMedia.template = xml`
- +
`; diff --git a/addons/web_unsplash/static/src/components/media_dialog/image_selector.js b/addons/web_unsplash/static/src/components/media_dialog/image_selector.js index c2750e9d0b2..c0eb0d93de1 100644 --- a/addons/web_unsplash/static/src/components/media_dialog/image_selector.js +++ b/addons/web_unsplash/static/src/components/media_dialog/image_selector.js @@ -1,6 +1,7 @@ /** @odoo-module **/ import { patch } from 'web.utils'; +import { KeepLast } from "@web/core/utils/concurrency"; import { MediaDialog, TABS } from '@web_editor/components/media_dialog/media_dialog'; import { ImageSelector } from '@web_editor/components/media_dialog/image_selector'; import { useService } from '@web/core/utils/hooks'; @@ -40,6 +41,7 @@ patch(ImageSelector.prototype, 'image_selector_unsplash', { setup() { this._super(); this.unsplash = useService('unsplash'); + this.keepLastUnsplash = new KeepLast(); this.state.unsplashRecords = []; this.state.isFetchingUnsplash = false; @@ -146,9 +148,11 @@ patch(ImageSelector.prototype, 'image_selector_unsplash', { async loadMore(...args) { await this._super(...args); - const { records, isMaxed } = await this.fetchUnsplashRecords(this.state.unsplashRecords.length); - this.state.unsplashRecords.push(...records); - this.state.isMaxed = isMaxed; + return this.keepLastUnsplash.add(this.fetchUnsplashRecords(this.state.unsplashRecords.length)).then(({ records, isMaxed }) => { + // This is never reached if another search or loadMore occurred. + this.state.unsplashRecords.push(...records); + this.state.isMaxed = isMaxed; + }); }, async search(...args) { @@ -162,9 +166,11 @@ patch(ImageSelector.prototype, 'image_selector_unsplash', { this.state.unsplashRecords = []; this.state.isMaxed = false; } - const { records, isMaxed } = await this.fetchUnsplashRecords(0); - this.state.unsplashRecords = records; - this.state.isMaxed = isMaxed; + return this.keepLastUnsplash.add(this.fetchUnsplashRecords(0)).then(({ records, isMaxed }) => { + // This is never reached if a new search occurred. + this.state.unsplashRecords = records; + this.state.isMaxed = isMaxed; + }); }, async onClickRecord(media) {