From 27976381f8b7afde4dc56e4512c2f38583eadab7 Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Thu, 13 Apr 2023 20:04:06 +0700 Subject: [PATCH] Updated GhContextMenu to handle freezing the SelectionList (#16624) refs https://github.com/TryGhost/Team/issues/2677 Fixed a bug that the current selection is deselected when clicking inside a modal when performing a context menu action. Refactored the context menu component and its usage in the posts list to improve the user experience and code quality. Introduced a `state` property and a `setState` method to the `gh-context-menu` component to handle different scenarios. Used the `gh-context-menu` component in the `posts-list/context-menu` component to simplify the modal and loading logic. Added a `#frozen` property and methods to the `selection-list` utility to prevent the selection of posts from changing while the context menu is active. --------- Co-authored-by: Simon Backx --- ghost/admin/app/components/gh-context-menu.js | 93 +++++++++++- .../app/components/posts-list/context-menu.js | 136 +++++++++--------- ghost/admin/app/utils/selection-list.js | 22 +++ 3 files changed, 179 insertions(+), 72 deletions(-) diff --git a/ghost/admin/app/components/gh-context-menu.js b/ghost/admin/app/components/gh-context-menu.js index e608ec7c42..4bca51893c 100644 --- a/ghost/admin/app/components/gh-context-menu.js +++ b/ghost/admin/app/components/gh-context-menu.js @@ -2,16 +2,69 @@ import Component from '@glimmer/component'; import SelectionList from '../utils/selection-list'; import {action} from '@ember/object'; import {inject as service} from '@ember/service'; +import {task} from 'ember-concurrency'; import {tracked} from '@glimmer/tracking'; export default class GhContextMenu extends Component { @service dropdown; + @service modals; @tracked isOpen = false; @tracked left = 0; @tracked top = 0; @tracked selectionList = new SelectionList(); + /** + * The current state of the context menu + * @type {'default'|'open'|'modal'|'loading'} + * default: default state + * open: menu open + * modal: modal open + * loading: performing an action + */ + state = states[0]; + + #originalConfirm = null; + #modal = null; + + setState(state) { + switch (state) { + case this.state: + return; + case 'default': + this.isOpen = false; + this.selectionList.unfreeze(); + this.#closeModal(); + this.state = state; + return; + case 'open': + if (this.state !== 'default') { + return; + } + this.isOpen = true; + this.selectionList.freeze(); + this.#closeModal(); + this.state = state; + return; + case 'modal': + if (this.state !== 'open') { + return; + } + this.isOpen = false; + this.selectionList.freeze(); + this.state = state; + return; + case 'loading': + if (this.state !== 'open' && this.state !== 'modal') { + return; + } + this.isOpen = false; + this.selectionList.freeze(); + this.state = state; + return; + } + } + get name() { return this.args.name; } @@ -36,12 +89,14 @@ export default class GhContextMenu extends Component { @action open() { - this.isOpen = true; + this.setState('open'); } @action close() { - this.isOpen = false; + if (this.state === 'open') { + this.setState('default'); + } } @action @@ -65,7 +120,7 @@ export default class GhContextMenu extends Component { } this.open(); - } else if (this.isOpen) { + } else { this.close(); } } @@ -74,4 +129,36 @@ export default class GhContextMenu extends Component { stopClicks(event) { event.stopPropagation(); } + + @task + *confirmWrapperTask(...args) { + this.setState('loading'); + let result = yield this.#originalConfirm.perform(...args); + this.#originalConfirm = null; + this.setState('default'); + return result; + } + + openModal(Modal, data) { + this.#originalConfirm = data.confirm; + data.confirm = this.confirmWrapperTask; + + this.setState('modal'); + + this.#modal = this.modals.open(Modal, data); + this.#modal.then(() => { + this.setState('default'); + }); + } + + #closeModal() { + this.#modal?.close(); + this.#modal = null; + } + + async performTask(taskObj) { + this.setState('loading'); + await taskObj.perform(); + this.setState('default'); + } } diff --git a/ghost/admin/app/components/posts-list/context-menu.js b/ghost/admin/app/components/posts-list/context-menu.js index 418c5cc298..cec33312b7 100644 --- a/ghost/admin/app/components/posts-list/context-menu.js +++ b/ghost/admin/app/components/posts-list/context-menu.js @@ -36,7 +36,6 @@ export default class PostsContextMenu extends Component { @service ghostPaths; @service session; @service infinity; - @service modals; @service store; @service notifications; @@ -55,10 +54,19 @@ export default class PostsContextMenu extends Component { return tpl(messages[type].multiple, {count: this.selectionList.count}); } + @action + async featurePosts() { + this.menu.performTask(this.featurePostsTask); + } + + @action + async unfeaturePosts() { + this.menu.performTask(this.unfeaturePostsTask); + } + @action async deletePosts() { - this.menu.close(); - await this.modals.open(DeletePostsModal, { + this.menu.openModal(DeletePostsModal, { selectionList: this.selectionList, confirm: this.deletePostsTask }); @@ -66,8 +74,7 @@ export default class PostsContextMenu extends Component { @action async unpublishPosts() { - this.menu.close(); - await this.modals.open(UnpublishPostsModal, { + await this.menu.openModal(UnpublishPostsModal, { selectionList: this.selectionList, confirm: this.unpublishPostsTask }); @@ -75,15 +82,14 @@ export default class PostsContextMenu extends Component { @action async editPostsAccess() { - this.menu.close(); - await this.modals.open(EditPostsAccessModal, { + this.menu.openModal(EditPostsAccessModal, { selectionList: this.selectionList, confirm: this.editPostsAccessTask }); } @task - *deletePostsTask(close) { + *deletePostsTask() { const deletedModels = this.selectionList.availableModels; yield this.performBulkDestroy(); this.notifications.showNotification(this.#getToastMessage('deleted'), {type: 'success'}); @@ -94,13 +100,11 @@ export default class PostsContextMenu extends Component { // Deleteobjects method from infintiymodel is broken for all models except the first page, so we cannot use this this.infinity.replace(this.selectionList.infinityModel, remainingModels); this.selectionList.clearSelection(); - close(); - return true; } @task - *unpublishPostsTask(close) { + *unpublishPostsTask() { const updatedModels = this.selectionList.availableModels; yield this.performBulkEdit('unpublish'); this.notifications.showNotification(this.#getToastMessage('unpublished'), {type: 'success'}); @@ -124,8 +128,6 @@ export default class PostsContextMenu extends Component { // Remove posts that no longer match the filter this.updateFilteredPosts(); - close(); - return true; } @@ -173,6 +175,58 @@ export default class PostsContextMenu extends Component { this.updateFilteredPosts(); close(); + } + + @task + *featurePostsTask() { + const updatedModels = this.selectionList.availableModels; + yield this.performBulkEdit('feature'); + + this.notifications.showNotification(this.#getToastMessage('featured'), {type: 'success'}); + + // Update the models on the client side + for (const post of updatedModels) { + // We need to do it this way to prevent marking the model as dirty + this.store.push({ + data: { + id: post.id, + type: 'post', + attributes: { + featured: true + } + } + }); + } + + // Remove posts that no longer match the filter + this.updateFilteredPosts(); + + return true; + } + + @task + *unfeaturePostsTask() { + const updatedModels = this.selectionList.availableModels; + yield this.performBulkEdit('unfeature'); + + this.notifications.showNotification(this.#getToastMessage('unfeatured'), {type: 'success'}); + + // Update the models on the client side + for (const post of updatedModels) { + // We need to do it this way to prevent marking the model as dirty + this.store.push({ + data: { + id: post.id, + type: 'post', + attributes: { + featured: false + } + } + }); + } + + // Remove posts that no longer match the filter + this.updateFilteredPosts(); return true; } @@ -223,60 +277,4 @@ export default class PostsContextMenu extends Component { } return false; } - - @action - async featurePosts() { - const updatedModels = this.selectionList.availableModels; - await this.performBulkEdit('feature'); - - this.notifications.showNotification(this.#getToastMessage('featured'), {type: 'success'}); - - // Update the models on the client side - for (const post of updatedModels) { - // We need to do it this way to prevent marking the model as dirty - this.store.push({ - data: { - id: post.id, - type: 'post', - attributes: { - featured: true - } - } - }); - } - - // Remove posts that no longer match the filter - this.updateFilteredPosts(); - - // Close the menu - this.menu.close(); - } - - @action - async unfeaturePosts() { - const updatedModels = this.selectionList.availableModels; - await this.performBulkEdit('unfeature'); - - this.notifications.showNotification(this.#getToastMessage('unfeatured'), {type: 'success'}); - - // Update the models on the client side - for (const post of updatedModels) { - // We need to do it this way to prevent marking the model as dirty - this.store.push({ - data: { - id: post.id, - type: 'post', - attributes: { - featured: false - } - } - }); - } - - // Remove posts that no longer match the filter - this.updateFilteredPosts(); - - // Close the menu - this.menu.close(); - } } diff --git a/ghost/admin/app/utils/selection-list.js b/ghost/admin/app/utils/selection-list.js index 78d6e4e584..c19f4b1c8b 100644 --- a/ghost/admin/app/utils/selection-list.js +++ b/ghost/admin/app/utils/selection-list.js @@ -10,10 +10,20 @@ export default class SelectionList { infinityModel; + #frozen = false; + constructor(infinityModel) { this.infinityModel = infinityModel ?? {content: []}; } + freeze() { + this.#frozen = true; + } + + unfreeze() { + this.#frozen = false; + } + /** * Returns an NQL filter for all items, not the selection */ @@ -90,6 +100,9 @@ export default class SelectionList { } toggleItem(id) { + if (this.#frozen) { + return; + } this.lastShiftSelectionGroup = new Set(); if (this.selectedIds.has(id)) { @@ -123,6 +136,9 @@ export default class SelectionList { * Select all items between the last selection or the first one if none */ shiftItem(id) { + if (this.#frozen) { + return; + } // Unselect last selected items for (const item of this.lastShiftSelectionGroup) { if (this.inverted) { @@ -181,12 +197,18 @@ export default class SelectionList { } selectAll() { + if (this.#frozen) { + return; + } this.selectedIds = new Set(); this.inverted = !this.inverted; this.lastSelectedId = null; } clearSelection() { + if (this.#frozen) { + return; + } this.selectedIds = new Set(); this.inverted = false; this.lastSelectedId = null;