From 44c8f4940eb21bac9e9a16b9e16043f47ae3416c Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Wed, 16 Mar 2022 16:41:24 +0200
Subject: [PATCH] FIX: Show bookmark options by default when editing (#16189)

These changes also update user bookmark preferences on the client side
immediately in case user creates more than a bookmark during the same
session.
---
 .../discourse/app/components/bookmark.js      | 34 +++++--------------
 .../app/templates/components/bookmark.hbs     | 22 ++++++------
 .../tests/acceptance/bookmarks-test.js        |  8 +++++
 .../common/components/bookmark-modal.scss     |  1 -
 4 files changed, 28 insertions(+), 37 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/bookmark.js b/app/assets/javascripts/discourse/app/components/bookmark.js
index 13035912ea6..8aa1dafb80d 100644
--- a/app/assets/javascripts/discourse/app/components/bookmark.js
+++ b/app/assets/javascripts/discourse/app/components/bookmark.js
@@ -55,11 +55,10 @@ export default Component.extend({
       userTimezone: this.currentUser.resolvedTimezone(this.currentUser),
       showOptions: false,
       _itsatrap: new ItsATrap(),
+      autoDeletePreference: this.model.autoDeletePreference || 0,
     });
 
     this.registerOnCloseHandler(this._onModalClose);
-
-    this._loadBookmarkOptions();
     this._bindKeyboardShortcuts();
 
     if (this.editingExistingBookmark) {
@@ -79,8 +78,8 @@ export default Component.extend({
 
     // we want to make sure the options panel opens so the user
     // knows they have set these options previously.
-    if (this.autoDeletePreference) {
-      this.toggleOptionsPanel();
+    if (this.model.id) {
+      this.set("showOptions", true);
     }
   },
 
@@ -98,21 +97,6 @@ export default Component.extend({
     }
   },
 
-  _loadBookmarkOptions() {
-    this.set(
-      "autoDeletePreference",
-      this.model.autoDeletePreference || this._preferredDeleteOption() || 0
-    );
-  },
-
-  _preferredDeleteOption() {
-    let preferred = localStorage.bookmarkDeleteOption;
-    if (preferred && preferred !== "") {
-      preferred = parseInt(preferred, 10);
-    }
-    return preferred;
-  },
-
   _bindKeyboardShortcuts() {
     KeyboardShortcuts.pause();
 
@@ -157,7 +141,10 @@ export default Component.extend({
       }
     }
 
-    localStorage.bookmarkDeleteOption = this.autoDeletePreference;
+    this.currentUser.set(
+      "bookmark_auto_delete_preference",
+      this.autoDeletePreference
+    );
 
     const data = {
       reminder_at: reminderAtISO,
@@ -347,12 +334,7 @@ export default Component.extend({
   },
 
   @action
-  toggleOptionsPanel() {
-    if (this.showOptions) {
-      $(".bookmark-options-panel").slideUp("fast");
-    } else {
-      $(".bookmark-options-panel").slideDown("fast");
-    }
+  toggleShowOptions() {
     this.toggleProperty("showOptions");
   },
 
diff --git a/app/assets/javascripts/discourse/app/templates/components/bookmark.hbs b/app/assets/javascripts/discourse/app/templates/components/bookmark.hbs
index 9d95a1ca0c9..e7b76e8bb9d 100644
--- a/app/assets/javascripts/discourse/app/templates/components/bookmark.hbs
+++ b/app/assets/javascripts/discourse/app/templates/components/bookmark.hbs
@@ -9,18 +9,20 @@
 
   <div class="control-group bookmark-name-wrap">
     {{input id="bookmark-name" value=model.name name="bookmark-name" class="bookmark-name" enter=(action "saveAndClose") placeholder=(i18n "post.bookmarks.name_placeholder") maxlength="100"}}
-    {{d-button icon="cog" action=(action "toggleOptionsPanel") class="bookmark-options-button" ariaLabel="post.bookmarks.options"}}
+    {{d-button icon="cog" action=(action "toggleShowOptions") class="bookmark-options-button" ariaLabel="post.bookmarks.options"}}
   </div>
 
-  <div class="bookmark-options-panel">
-    <label class="control-label" for="bookmark_auto_delete_preference">{{i18n "bookmarks.auto_delete_preference.label"}}</label>
-    {{combo-box
-    content=autoDeletePreferences
-    value=autoDeletePreference
-    class="bookmark-option-selector"
-    onChange=(action (mut autoDeletePreference))
-    }}
-  </div>
+  {{#if showOptions}}
+    <div class="bookmark-options-panel">
+      <label class="control-label" for="bookmark_auto_delete_preference">{{i18n "bookmarks.auto_delete_preference.label"}}</label>
+      {{combo-box
+      content=autoDeletePreferences
+      value=autoDeletePreference
+      class="bookmark-option-selector"
+      onChange=(action (mut autoDeletePreference))
+      }}
+    </div>
+  {{/if}}
 
   {{#if showExistingReminderAt }}
     <div class="alert alert-info existing-reminder-at-alert">
diff --git a/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js b/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js
index a1704a76266..177baae964c 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js
@@ -11,6 +11,7 @@ import selectKit from "discourse/tests/helpers/select-kit-helper";
 import { test } from "qunit";
 import topicFixtures from "discourse/tests/fixtures/topic";
 import { cloneJSON } from "discourse-common/lib/object";
+import User from "discourse/models/user";
 
 async function openBookmarkModal(postNumber = 1) {
   if (exists(`#post_${postNumber} button.show-more-actions`)) {
@@ -154,6 +155,10 @@ acceptance("Bookmarking", function (needs) {
   test("Opening the options panel and remembering the option", async function (assert) {
     await visit("/t/internationalization-localization/280");
     await openBookmarkModal();
+    assert.notOk(
+      exists(".bookmark-options-panel"),
+      "it should not open the options panel by default"
+    );
     await click(".bookmark-options-button");
     assert.ok(
       exists(".bookmark-options-panel"),
@@ -162,6 +167,9 @@ acceptance("Bookmarking", function (needs) {
     await selectKit(".bookmark-option-selector").expand();
     await selectKit(".bookmark-option-selector").selectRowByValue(1);
     await click("#save-bookmark");
+
+    assert.equal(User.current().bookmark_auto_delete_preference, "1");
+
     await openEditBookmarkModal();
 
     assert.ok(
diff --git a/app/assets/stylesheets/common/components/bookmark-modal.scss b/app/assets/stylesheets/common/components/bookmark-modal.scss
index def10d556ac..bb638c5b1a7 100644
--- a/app/assets/stylesheets/common/components/bookmark-modal.scss
+++ b/app/assets/stylesheets/common/components/bookmark-modal.scss
@@ -60,7 +60,6 @@
     }
 
     margin-bottom: 18px;
-    display: none;
 
     label {
       display: flex;