From fcbb6c414338c4aa3cc6b0708984918541ee0389 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Mon, 25 Jan 2021 09:35:13 +1100 Subject: [PATCH] FIX: remove rendering UX from bookmark model (#11765) Fix for `bookmark.js` model. Most logic was moved to `topic` controller --- .../app/components/bookmark-local-date.js | 57 ++++++++++ .../app/components/topic-list-item.js | 6 - .../discourse/app/controllers/bookmark.js | 56 +++++----- .../discourse/app/controllers/topic.js | 105 +++++++++++++++++- .../javascripts/discourse/app/models/post.js | 69 +++--------- .../javascripts/discourse/app/models/topic.js | 70 +----------- .../components/bookmark-local-date.hbs | 6 + .../app/templates/modal/bookmark.hbs | 7 +- .../initializers/new-user-narrative.js.es6 | 14 +-- 9 files changed, 223 insertions(+), 167 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/components/bookmark-local-date.js create mode 100644 app/assets/javascripts/discourse/app/templates/components/bookmark-local-date.hbs diff --git a/app/assets/javascripts/discourse/app/components/bookmark-local-date.js b/app/assets/javascripts/discourse/app/components/bookmark-local-date.js new file mode 100644 index 00000000000..d89c2000737 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/bookmark-local-date.js @@ -0,0 +1,57 @@ +import Component from "@ember/component"; +import I18n from "I18n"; +import { action } from "@ember/object"; +import { getOwner } from "discourse-common/lib/get-owner"; +import { or } from "@ember/object/computed"; + +export default Component.extend({ + tagName: "", + + init() { + this._super(...arguments); + + this.loadLocalDates(); + }, + + get postLocalDateFormatted() { + return this.postLocalDate().format(I18n.t("dates.long_no_year")); + }, + + showPostLocalDate: or("postDetectedLocalDate", "postDetectedLocalTime"), + + loadLocalDates() { + let postEl = document.querySelector(`[data-post-id="${this.postId}"]`); + let localDateEl = null; + if (postEl) { + localDateEl = postEl.querySelector(".discourse-local-date"); + } + + this.setProperties({ + postDetectedLocalDate: localDateEl ? localDateEl.dataset.date : null, + postDetectedLocalTime: localDateEl ? localDateEl.dataset.time : null, + postDetectedLocalTimezone: localDateEl + ? localDateEl.dataset.timezone + : null, + }); + }, + + postLocalDate() { + const bookmarkController = getOwner(this).lookup("controller:bookmark"); + let parsedPostLocalDate = bookmarkController._parseCustomDateTime( + this.postDetectedLocalDate, + this.postDetectedLocalTime, + this.postDetectedLocalTimezone + ); + + if (!this.postDetectedLocalTime) { + return bookmarkController.startOfDay(parsedPostLocalDate); + } + + return parsedPostLocalDate; + }, + + @action + setReminder() { + return this.onChange(this.postLocalDate()); + }, +}); diff --git a/app/assets/javascripts/discourse/app/components/topic-list-item.js b/app/assets/javascripts/discourse/app/components/topic-list-item.js index 22d67f2d002..2efef6a1f3d 100644 --- a/app/assets/javascripts/discourse/app/components/topic-list-item.js +++ b/app/assets/javascripts/discourse/app/components/topic-list-item.js @@ -226,12 +226,6 @@ export default Component.extend({ return this.unhandledRowClick(e, topic); }, - actions: { - toggleBookmark() { - this.topic.toggleBookmark().finally(() => this.renderTopicListItem()); - }, - }, - unhandledRowClick() {}, navigateToTopic, diff --git a/app/assets/javascripts/discourse/app/controllers/bookmark.js b/app/assets/javascripts/discourse/app/controllers/bookmark.js index 5cab58bd0a4..ec0e0010bbc 100644 --- a/app/assets/javascripts/discourse/app/controllers/bookmark.js +++ b/app/assets/javascripts/discourse/app/controllers/bookmark.js @@ -1,5 +1,4 @@ import { REMINDER_TYPES, formattedReminderTime } from "discourse/lib/bookmark"; -import { and, or } from "@ember/object/computed"; import { isEmpty, isPresent } from "@ember/utils"; import { next, schedule } from "@ember/runloop"; import { AUTO_DELETE_PREFERENCES } from "discourse/models/bookmark"; @@ -10,6 +9,7 @@ import ModalFunctionality from "discourse/mixins/modal-functionality"; import { Promise } from "rsvp"; import { action } from "@ember/object"; import { ajax } from "discourse/lib/ajax"; +import { and } from "@ember/object/computed"; import bootbox from "bootbox"; import discourseComputed from "discourse-common/utils/decorators"; import { popupAjaxError } from "discourse/lib/ajax-error"; @@ -62,9 +62,7 @@ export default Controller.extend(ModalFunctionality, { customReminderTime: null, lastCustomReminderDate: null, lastCustomReminderTime: null, - postDetectedLocalDate: null, - postDetectedLocalTime: null, - postDetectedLocalTimezone: null, + postLocalDate: null, mouseTrap: null, userTimezone: null, showOptions: false, @@ -95,6 +93,8 @@ export default Controller.extend(ModalFunctionality, { this._initializeExistingBookmarkData(); } + this.loadLocalDates(); + schedule("afterRender", () => { if (this.site.isMobileDevice) { document.getElementById("bookmark-name").blur(); @@ -240,11 +240,6 @@ export default Controller.extend(ModalFunctionality, { showLastCustom: and("lastCustomReminderTime", "lastCustomReminderDate"), - showPostLocalDate: or( - "model.postDetectedLocalDate", - "model.postDetectedLocalTime" - ), - get showLaterToday() { let later = this.laterToday(); return ( @@ -302,8 +297,22 @@ export default Controller.extend(ModalFunctionality, { return this.nextMonth().format(I18n.t("dates.long_no_year")); }, - get postLocalDateFormatted() { - return this.postLocalDate().format(I18n.t("dates.long_no_year")); + loadLocalDates() { + let postEl = document.querySelector( + `[data-post-id="${this.model.postId}"]` + ); + let localDateEl = null; + if (postEl) { + localDateEl = postEl.querySelector(".discourse-local-date"); + } + + if (localDateEl) { + this.setProperties({ + postDetectedLocalDate: localDateEl.dataset.date, + postDetectedLocalTime: localDateEl.dataset.time, + postDetectedLocalTimezone: localDateEl.dataset.timezone, + }); + } }, @discourseComputed("userTimezone") @@ -442,7 +451,7 @@ export default Controller.extend(ModalFunctionality, { case REMINDER_TYPES.LAST_CUSTOM: return this.parsedLastCustomReminderDatetime; case REMINDER_TYPES.POST_LOCAL_DATE: - return this.postLocalDate(); + return this.postLocalDate; } }, @@ -454,20 +463,6 @@ export default Controller.extend(ModalFunctionality, { return this.startOfDay(this.now().add(1, "month")); }, - postLocalDate() { - let parsedPostLocalDate = this._parseCustomDateTime( - this.model.postDetectedLocalDate, - this.model.postDetectedLocalTime, - this.model.postDetectedLocalTimezone - ); - - if (!this.model.postDetectedLocalTime) { - return this.startOfDay(parsedPostLocalDate); - } - - return parsedPostLocalDate; - }, - tomorrow() { return this.startOfDay(this.now().add(1, "day")); }, @@ -572,4 +567,13 @@ export default Controller.extend(ModalFunctionality, { return this.saveAndClose(); } }, + + @action + selectPostLocalDate(date) { + this.setProperties({ + selectedReminderType: this.reminderTypes.POST_LOCAL_DATE, + postLocalDate: date, + }); + return this.saveAndClose(); + }, }); diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index e050aca6eaf..7240ee39e97 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/app/assets/javascripts/discourse/app/controllers/topic.js @@ -702,9 +702,9 @@ export default Controller.extend(bufferedProperty("model"), { if (!this.currentUser) { return bootbox.alert(I18n.t("bookmarks.not_bookmarked")); } else if (post) { - return post.toggleBookmark(); + return this._togglePostBookmark(post); } else { - return this.model.toggleBookmark().then((changedIds) => { + return this._toggleTopicBookmark(this.model).then((changedIds) => { if (!changedIds) { return; } @@ -1167,6 +1167,107 @@ export default Controller.extend(bufferedProperty("model"), { } }, + _togglePostBookmark(post) { + return new Promise((resolve) => { + let modalController = showModal("bookmark", { + model: { + postId: post.id, + id: post.bookmark_id, + reminderAt: post.bookmark_reminder_at, + autoDeletePreference: post.bookmark_auto_delete_preference, + name: post.bookmark_name, + }, + title: post.bookmark_id + ? "post.bookmarks.edit" + : "post.bookmarks.create", + modalClass: "bookmark-with-reminder", + }); + modalController.setProperties({ + onCloseWithoutSaving: () => { + resolve({ closedWithoutSaving: true }); + post.appEvents.trigger("post-stream:refresh", { id: post.id }); + }, + afterSave: (savedData) => { + post.createBookmark(savedData); + resolve({ closedWithoutSaving: false }); + }, + afterDelete: (topicBookmarked) => { + post.deleteBookmark(topicBookmarked); + }, + }); + }); + }, + + _toggleTopicBookmark() { + if (this.model.bookmarking) { + return Promise.resolve(); + } + this.model.set("bookmarking", true); + const bookmark = !this.model.bookmarked; + let posts = this.model.postStream.posts; + + return this.model.firstPost().then((firstPost) => { + const toggleBookmarkOnServer = () => { + if (bookmark) { + return this._togglePostBookmark(firstPost).then((opts) => { + this.model.set("bookmarking", false); + if (opts && opts.closedWithoutSaving) { + return; + } + return this.model.afterTopicBookmarked(firstPost); + }); + } else { + return this.model + .deleteBookmark() + .then(() => { + this.model.toggleProperty("bookmarked"); + this.model.set("bookmark_reminder_at", null); + let clearedBookmarkProps = { + bookmarked: false, + bookmark_id: null, + bookmark_name: null, + bookmark_reminder_at: null, + }; + if (posts) { + const updated = []; + posts.forEach((post) => { + if (post.bookmarked) { + post.setProperties(clearedBookmarkProps); + updated.push(post.id); + } + }); + firstPost.setProperties(clearedBookmarkProps); + return updated; + } + }) + .catch(popupAjaxError) + .finally(() => this.model.set("bookmarking", false)); + } + }; + + const unbookmarkedPosts = []; + if (!bookmark && posts) { + posts.forEach( + (post) => post.bookmarked && unbookmarkedPosts.push(post) + ); + } + + return new Promise((resolve) => { + if (unbookmarkedPosts.length > 1) { + bootbox.confirm( + I18n.t("bookmarks.confirm_clear"), + I18n.t("no_value"), + I18n.t("yes_value"), + (confirmed) => + confirmed ? toggleBookmarkOnServer().then(resolve) : resolve() + ); + } else { + toggleBookmarkOnServer().then(resolve); + } + }); + }); + }, + togglePinnedState() { this.send("togglePinnedForUser"); }, diff --git a/app/assets/javascripts/discourse/app/models/post.js b/app/assets/javascripts/discourse/app/models/post.js index 5a8acd7c963..4c3a671ddff 100644 --- a/app/assets/javascripts/discourse/app/models/post.js +++ b/app/assets/javascripts/discourse/app/models/post.js @@ -16,7 +16,6 @@ import { popupAjaxError } from "discourse/lib/ajax-error"; import { postUrl } from "discourse/lib/utilities"; import { propertyEqual } from "discourse/lib/computed"; import { resolveShareUrl } from "discourse/helpers/share-url"; -import showModal from "discourse/lib/show-modal"; import { userPath } from "discourse/lib/url"; const Post = RestModel.extend({ @@ -304,58 +303,24 @@ const Post = RestModel.extend({ return ajax(`/posts/${this.id}/unhide`, { type: "PUT" }); }, - toggleBookmark() { - let postEl = document.querySelector(`[data-post-id="${this.id}"]`); - let localDateEl = null; - if (postEl) { - localDateEl = postEl.querySelector(".discourse-local-date"); - } - - return new Promise((resolve) => { - let controller = showModal("bookmark", { - model: { - postId: this.id, - id: this.bookmark_id, - reminderAt: this.bookmark_reminder_at, - autoDeletePreference: this.bookmark_auto_delete_preference, - name: this.bookmark_name, - postDetectedLocalDate: localDateEl ? localDateEl.dataset.date : null, - postDetectedLocalTime: localDateEl ? localDateEl.dataset.time : null, - postDetectedLocalTimezone: localDateEl - ? localDateEl.dataset.timezone - : null, - }, - title: this.bookmark_id - ? "post.bookmarks.edit" - : "post.bookmarks.create", - modalClass: "bookmark-with-reminder", - }); - controller.setProperties({ - onCloseWithoutSaving: () => { - resolve({ closedWithoutSaving: true }); - this.appEvents.trigger("post-stream:refresh", { id: this.id }); - }, - afterSave: (savedData) => { - this.setProperties({ - "topic.bookmarked": true, - bookmarked: true, - bookmark_reminder_at: savedData.reminderAt, - bookmark_reminder_type: savedData.reminderType, - bookmark_auto_delete_preference: savedData.autoDeletePreference, - bookmark_name: savedData.name, - bookmark_id: savedData.id, - }); - resolve({ closedWithoutSaving: false }); - this.appEvents.trigger("page:bookmark-post-toggled", this); - this.appEvents.trigger("post-stream:refresh", { id: this.id }); - }, - afterDelete: (topicBookmarked) => { - this.set("topic.bookmarked", topicBookmarked); - this.clearBookmark(); - this.appEvents.trigger("page:bookmark-post-toggled", this); - }, - }); + createBookmark(data) { + this.setProperties({ + "topic.bookmarked": true, + bookmarked: true, + bookmark_reminder_at: data.reminderAt, + bookmark_reminder_type: data.reminderType, + bookmark_auto_delete_preference: data.autoDeletePreference, + bookmark_name: data.name, + bookmark_id: data.id, }); + this.appEvents.trigger("page:bookmark-post-toggled", this); + this.appEvents.trigger("post-stream:refresh", { id: this.id }); + }, + + deleteBookmark(bookmarked) { + this.set("topic.bookmarked", bookmarked); + this.clearBookmark(); + this.appEvents.trigger("page:bookmark-post-toggled", this); }, clearBookmark() { diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js index 8796bc4261e..6de4b88f73e 100644 --- a/app/assets/javascripts/discourse/app/models/topic.js +++ b/app/assets/javascripts/discourse/app/models/topic.js @@ -11,7 +11,6 @@ import Session from "discourse/models/session"; import Site from "discourse/models/site"; import User from "discourse/models/user"; import { ajax } from "discourse/lib/ajax"; -import bootbox from "bootbox"; import { deepMerge } from "discourse-common/lib/object"; import discourseComputed from "discourse-common/utils/decorators"; import { emojiUnescape } from "discourse/lib/text"; @@ -404,73 +403,8 @@ const Topic = RestModel.extend({ } }, - toggleBookmark() { - if (this.bookmarking) { - return Promise.resolve(); - } - this.set("bookmarking", true); - const bookmark = !this.bookmarked; - let posts = this.postStream.posts; - - return this.firstPost().then((firstPost) => { - const toggleBookmarkOnServer = () => { - if (bookmark) { - return firstPost.toggleBookmark().then((opts) => { - this.set("bookmarking", false); - if (opts.closedWithoutSaving) { - return; - } - return this.afterTopicBookmarked(firstPost); - }); - } else { - return ajax(`/t/${this.id}/remove_bookmarks`, { type: "PUT" }) - .then(() => { - this.toggleProperty("bookmarked"); - this.set("bookmark_reminder_at", null); - let clearedBookmarkProps = { - bookmarked: false, - bookmark_id: null, - bookmark_name: null, - bookmark_reminder_at: null, - }; - if (posts) { - const updated = []; - posts.forEach((post) => { - if (post.bookmarked) { - post.setProperties(clearedBookmarkProps); - updated.push(post.id); - } - }); - firstPost.setProperties(clearedBookmarkProps); - return updated; - } - }) - .catch(popupAjaxError) - .finally(() => this.set("bookmarking", false)); - } - }; - - const unbookmarkedPosts = []; - if (!bookmark && posts) { - posts.forEach( - (post) => post.bookmarked && unbookmarkedPosts.push(post) - ); - } - - return new Promise((resolve) => { - if (unbookmarkedPosts.length > 1) { - bootbox.confirm( - I18n.t("bookmarks.confirm_clear"), - I18n.t("no_value"), - I18n.t("yes_value"), - (confirmed) => - confirmed ? toggleBookmarkOnServer().then(resolve) : resolve() - ); - } else { - toggleBookmarkOnServer().then(resolve); - } - }); - }); + deleteBookmark() { + return ajax(`/t/${this.id}/remove_bookmarks`, { type: "PUT" }); }, createGroupInvite(group) { diff --git a/app/assets/javascripts/discourse/app/templates/components/bookmark-local-date.hbs b/app/assets/javascripts/discourse/app/templates/components/bookmark-local-date.hbs new file mode 100644 index 00000000000..a30363826de --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/components/bookmark-local-date.hbs @@ -0,0 +1,6 @@ +{{#if showPostLocalDate}} + {{#tap-tile icon="globe-americas" tileId=tileId activeTile=activeTile onChange=(action "setReminder")}} +
{{i18n "bookmarks.reminders.post_local_date"}}
+
{{postLocalDateFormatted}}
+ {{/tap-tile}} +{{/if}} diff --git a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs index ba19e49d429..46049bb9b7a 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs @@ -65,12 +65,7 @@
{{i18n "bookmarks.reminders.next_month"}}
{{nextMonthFormatted}}
{{/tap-tile}} - {{#if showPostLocalDate}} - {{#tap-tile icon="globe-americas" tileId=reminderTypes.POST_LOCAL_DATE activeTile=grid.activeTile onChange=(action "selectReminderType")}} -
{{i18n "bookmarks.reminders.post_local_date"}}
-
{{postLocalDateFormatted}}
- {{/tap-tile}} - {{/if}} + {{bookmark-local-date postId=model.postId tileId=reminderTypes.POST_LOCAL_DATE activeTile=grid.activeTile onChange=(action "selectPostLocalDate")}} {{#tap-tile icon="calendar-alt" tileId=reminderTypes.CUSTOM activeTile=grid.activeTile onChange=(action "selectReminderType")}}
{{i18n "bookmarks.reminders.custom"}}
{{/tap-tile}} diff --git a/plugins/discourse-narrative-bot/assets/javascripts/initializers/new-user-narrative.js.es6 b/plugins/discourse-narrative-bot/assets/javascripts/initializers/new-user-narrative.js.es6 index 3cac3b63e0d..156c4dd0cd2 100644 --- a/plugins/discourse-narrative-bot/assets/javascripts/initializers/new-user-narrative.js.es6 +++ b/plugins/discourse-narrative-bot/assets/javascripts/initializers/new-user-narrative.js.es6 @@ -13,26 +13,26 @@ function initialize(api) { }, }); - api.modifyClass("model:post", { - toggleBookmark() { + api.modifyClass("controller:topic", { + _togglePostBookmark(post) { // if we are talking to discobot then any bookmarks should just // be created without reminder options, to streamline the new user // narrative. const discobotUserId = -2; - if (this.user_id === discobotUserId && !this.bookmarked) { + if (post.user_id === discobotUserId && !post.bookmarked) { return ajax("/bookmarks", { type: "POST", - data: { post_id: this.id }, + data: { post_id: post.id }, }).then((response) => { - this.setProperties({ + post.setProperties({ "topic.bookmarked": true, bookmarked: true, bookmark_id: response.id, }); - this.appEvents.trigger("post-stream:refresh", { id: this.id }); + post.appEvents.trigger("post-stream:refresh", { id: this.id }); }); } - return this._super(); + return this._super(post); }, });