From 1d131fcaff53f43a52801c54a39f57d51065a718 Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Tue, 19 Oct 2021 10:32:20 +1000
Subject: [PATCH] FIX: Clarify None Needed option when editing bookmarks
 (#14633)

This commit makes the following change to the Edit Bookmark
modal window for clarity:

* If the user is editing an existing bookmark without a reminder set,
hide the "none needed" option. This will draw more attention to the
delete button.
* If the user is editing an existing bookmark with a reminder set for the
future, change the "none needed" option to say "remove reminder, keep bookmark"

To do this, I needed to provide an option to override the labels
for time shortcuts in certain cases, so I could keep the NONE shortcut
but have the different wording.
---
 .../discourse/app/components/bookmark.js      | 28 ++++++++++++++++++-
 .../app/components/time-shortcut-picker.js    | 16 ++++++++++-
 .../app/templates/components/bookmark.hbs     |  2 ++
 config/locales/client.en.yml                  |  1 +
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/bookmark.js b/app/assets/javascripts/discourse/app/components/bookmark.js
index b6354419d66..35b8a31a4ee 100644
--- a/app/assets/javascripts/discourse/app/components/bookmark.js
+++ b/app/assets/javascripts/discourse/app/components/bookmark.js
@@ -265,7 +265,7 @@ export default Component.extend({
   showDelete: notEmpty("model.id"),
   userHasTimezoneSet: notEmpty("userTimezone"),
   editingExistingBookmark: and("model", "model.id"),
-  existingBookmarkHasReminder: and("model", "model.reminderAt"),
+  existingBookmarkHasReminder: and("model", "model.id", "model.reminderAt"),
 
   @discourseComputed("postDetectedLocalDate", "postDetectedLocalTime")
   showPostLocalDate(postDetectedLocalDate, postDetectedLocalTime) {
@@ -311,6 +311,32 @@ export default Component.extend({
     return customOptions;
   },
 
+  @discourseComputed("existingBookmarkHasReminder")
+  customTimeShortcutLabels(existingBookmarkHasReminder) {
+    const labels = {};
+    if (existingBookmarkHasReminder) {
+      labels[TIME_SHORTCUT_TYPES.NONE] =
+        "bookmarks.remove_reminder_keep_bookmark";
+    }
+    return labels;
+  },
+
+  @discourseComputed("editingExistingBookmark", "existingBookmarkHasReminder")
+  hiddenTimeShortcutOptions(
+    editingExistingBookmark,
+    existingBookmarkHasReminder
+  ) {
+    if (!editingExistingBookmark) {
+      return [];
+    }
+
+    if (!existingBookmarkHasReminder) {
+      return [TIME_SHORTCUT_TYPES.NONE];
+    }
+
+    return [];
+  },
+
   @discourseComputed()
   additionalTimeShortcutOptions() {
     let additional = [];
diff --git a/app/assets/javascripts/discourse/app/components/time-shortcut-picker.js b/app/assets/javascripts/discourse/app/components/time-shortcut-picker.js
index 62869a39dd6..a2ff789e432 100644
--- a/app/assets/javascripts/discourse/app/components/time-shortcut-picker.js
+++ b/app/assets/javascripts/discourse/app/components/time-shortcut-picker.js
@@ -79,6 +79,7 @@ export default Component.extend({
       additionalOptionsToShow: this.additionalOptionsToShow || [],
       hiddenOptions: this.hiddenOptions || [],
       customOptions: this.customOptions || [],
+      customLabels: this.customLabels || {},
     });
 
     if (this.prefilledDatetime) {
@@ -170,9 +171,16 @@ export default Component.extend({
     "additionalOptionsToShow",
     "hiddenOptions",
     "customOptions",
+    "customLabels",
     "userTimezone"
   )
-  options(additionalOptionsToShow, hiddenOptions, customOptions, userTimezone) {
+  options(
+    additionalOptionsToShow,
+    hiddenOptions,
+    customOptions,
+    customLabels,
+    userTimezone
+  ) {
     this._loadLastUsedCustomDatetime();
 
     let options = defaultShortcutOptions(userTimezone);
@@ -226,6 +234,12 @@ export default Component.extend({
       });
     }
 
+    options.forEach((option) => {
+      if (customLabels[option.id]) {
+        option.label = customLabels[option.id];
+      }
+    });
+
     return options;
   },
 
diff --git a/app/assets/javascripts/discourse/app/templates/components/bookmark.hbs b/app/assets/javascripts/discourse/app/templates/components/bookmark.hbs
index 00119efd9dc..bc75391e1b3 100644
--- a/app/assets/javascripts/discourse/app/templates/components/bookmark.hbs
+++ b/app/assets/javascripts/discourse/app/templates/components/bookmark.hbs
@@ -39,6 +39,8 @@
         prefilledDatetime=prefilledDatetime
         onTimeSelected=(action "onTimeSelected")
         customOptions=customTimeShortcutOptions
+        hiddenOptions=hiddenTimeShortcutOptions
+        customLabels=customTimeShortcutLabels
         additionalOptionsToShow=additionalTimeShortcutOptions
         _itsatrap=_itsatrap
       }}
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 6e919bd4acf..2054b24c4de 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -312,6 +312,7 @@ en:
     bookmarks:
       created: "You've bookmarked this post. %{name}"
       not_bookmarked: "bookmark this post"
+      remove_reminder_keep_bookmark: "Remove reminder and keep bookmark"
       created_with_reminder: "You've bookmarked this post with a reminder %{date}. %{name}"
       remove: "Remove Bookmark"
       delete: "Delete Bookmark"