From f4f3e8c4019efc7c98f2dc09834e59dde77ffdda Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Tue, 14 Jul 2020 14:02:13 +1000
Subject: [PATCH] FIX: Various improvements to bookmark modal UI (#10225)

* Do not autofocus name input on mobile
* Improve code for formatted reminder type times to not be computed, so the modal times update correctly
* Change wording of "Next Monday" to "Monday" for all days except when today is Monday
---
 .../discourse/app/controllers/bookmark.js     | 38 +++++++++++--------
 .../app/templates/modal/bookmark.hbs          |  2 +-
 config/locales/client.en.yml                  |  3 +-
 test/javascripts/acceptance/bookmarks-test.js | 26 +++++++++++++
 test/javascripts/controllers/bookmark-test.js |  5 ++-
 5 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/controllers/bookmark.js b/app/assets/javascripts/discourse/app/controllers/bookmark.js
index 05a4c26b530..c2a6b5585ac 100644
--- a/app/assets/javascripts/discourse/app/controllers/bookmark.js
+++ b/app/assets/javascripts/discourse/app/controllers/bookmark.js
@@ -1,4 +1,5 @@
 import I18n from "I18n";
+import { schedule } from "@ember/runloop";
 import { and } from "@ember/object/computed";
 import { next } from "@ember/runloop";
 import { action } from "@ember/object";
@@ -89,6 +90,12 @@ export default Controller.extend(ModalFunctionality, {
     if (this._editingExistingBookmark()) {
       this._initializeExistingBookmarkData();
     }
+
+    schedule("afterRender", () => {
+      if (this.site.isMobileDevice) {
+        document.getElementById("bookmark-name").blur();
+      }
+    });
   },
 
   /**
@@ -212,8 +219,7 @@ export default Controller.extend(ModalFunctionality, {
 
   showLastCustom: and("lastCustomReminderTime", "lastCustomReminderDate"),
 
-  @discourseComputed()
-  showLaterToday() {
+  get showLaterToday() {
     let later = this.laterToday();
     return (
       !later.isSame(this.tomorrow(), "date") &&
@@ -221,8 +227,7 @@ export default Controller.extend(ModalFunctionality, {
     );
   },
 
-  @discourseComputed()
-  showLaterThisWeek() {
+  get showLaterThisWeek() {
     return this.now().day() < MOMENT_THURSDAY;
   },
 
@@ -238,35 +243,36 @@ export default Controller.extend(ModalFunctionality, {
     return formattedReminderTime(existingReminderAt, this.userTimezone);
   },
 
-  @discourseComputed()
-  startNextBusinessWeekFormatted() {
+  get startNextBusinessWeekLabel() {
+    if (this.now().day() === MOMENT_MONDAY) {
+      return I18n.t("bookmarks.reminders.start_of_next_business_week_alt");
+    }
+    return I18n.t("bookmarks.reminders.start_of_next_business_week");
+  },
+
+  get startNextBusinessWeekFormatted() {
     return this.nextWeek()
       .day(MOMENT_MONDAY)
       .format(I18n.t("dates.long_no_year"));
   },
 
-  @discourseComputed()
-  laterTodayFormatted() {
+  get laterTodayFormatted() {
     return this.laterToday().format(I18n.t("dates.time"));
   },
 
-  @discourseComputed()
-  tomorrowFormatted() {
+  get tomorrowFormatted() {
     return this.tomorrow().format(I18n.t("dates.time_short_day"));
   },
 
-  @discourseComputed()
-  nextWeekFormatted() {
+  get nextWeekFormatted() {
     return this.nextWeek().format(I18n.t("dates.long_no_year"));
   },
 
-  @discourseComputed()
-  laterThisWeekFormatted() {
+  get laterThisWeekFormatted() {
     return this.laterThisWeek().format(I18n.t("dates.time_short_day"));
   },
 
-  @discourseComputed()
-  nextMonthFormatted() {
+  get nextMonthFormatted() {
     return this.nextMonth().format(I18n.t("dates.long_no_year"));
   },
 
diff --git a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs
index 17f41b174ef..87d75375c14 100644
--- a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs
+++ b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs
@@ -51,7 +51,7 @@
             {{/tap-tile}}
           {{/if}}
           {{#tap-tile icon="briefcase" tileId=reminderTypes.START_OF_NEXT_BUSINESS_WEEK activeTile=grid.activeTile onChange=(action "selectReminderType")}}
-            <div class="tap-tile-title">{{i18n "bookmarks.reminders.start_of_next_business_week"}}</div>
+            <div class="tap-tile-title">{{startNextBusinessWeekLabel}}</div>
             <div class="tap-tile-date">{{startNextBusinessWeekFormatted}}</div>
           {{/tap-tile}}
           {{#tap-tile icon="far-clock" tileId=reminderTypes.NEXT_WEEK activeTile=grid.activeTile onChange=(action "selectReminderType")}}
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 4b13817ba8f..da71b0f4af3 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -327,7 +327,8 @@ en:
         tomorrow: "Tomorrow"
         next_week: "Next week"
         later_this_week: "Later this week"
-        start_of_next_business_week: "Next Monday"
+        start_of_next_business_week: "Monday"
+        start_of_next_business_week_alt: "Next Monday"
         next_month: "Next month"
         custom: "Custom date and time"
         last_custom: "Last"
diff --git a/test/javascripts/acceptance/bookmarks-test.js b/test/javascripts/acceptance/bookmarks-test.js
index db94c6a8f3c..59eadc0d126 100644
--- a/test/javascripts/acceptance/bookmarks-test.js
+++ b/test/javascripts/acceptance/bookmarks-test.js
@@ -230,6 +230,32 @@ test("Editing a bookmark", async assert => {
   assert.verifySteps(["tomorrow"]);
 });
 
+test("Opening bookmark modal on desktop should auto-focus name", async assert => {
+  mockSuccessfulBookmarkPost(assert);
+
+  await visit("/t/internationalization-localization/280");
+  await openBookmarkModal();
+
+  assert.equal($("#bookmark-name").is(":focus"), true);
+});
+
+acceptance("Bookmarking - Mobile", {
+  loggedIn: true,
+  mobileView: true,
+  afterEach() {
+    sandbox.restore();
+  }
+});
+
+test("Opening bookmark modal on mobile should not auto-focus name", async assert => {
+  mockSuccessfulBookmarkPost(assert);
+
+  await visit("/t/internationalization-localization/280");
+  await openBookmarkModal();
+
+  assert.equal($("#bookmark-name").is(":focus"), false);
+});
+
 QUnit.skip(
   "Editing a bookmark that has a Later Today reminder, and it is before 6pm today",
   async assert => {
diff --git a/test/javascripts/controllers/bookmark-test.js b/test/javascripts/controllers/bookmark-test.js
index 0474215f4ab..1e7c78491b4 100644
--- a/test/javascripts/controllers/bookmark-test.js
+++ b/test/javascripts/controllers/bookmark-test.js
@@ -9,7 +9,10 @@ moduleFor("controller:bookmark", {
   beforeEach() {
     logIn();
     KeyboardShortcutInitializer.initialize(Discourse.__container__);
-    BookmarkController = this.subject({ currentUser: User.current() });
+    BookmarkController = this.subject({
+      currentUser: User.current(),
+      site: { isMobileDevice: false }
+    });
     BookmarkController.onShow();
   },