FIX: Clarify None Needed option when editing bookmarks (#14633)

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.

diff --git a/app/assets/javascripts/discourse/app/components/bookmark.js b/app/assets/javascripts/discourse/app/components/bookmark.js
index b635441..35b8a31 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 62869a3..a2ff789 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 00119ef..bc75391 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 6e919bd..2054b24 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"

GitHub sha: 1d131fcaff53f43a52801c54a39f57d51065a718

This commit appears in #14633 which was approved by pmusaraj. It was merged by martin.