From 00255d0bd215d4dec831c16b460f1d301c1fc6dc Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Tue, 15 Jun 2021 16:54:00 +0400 Subject: [PATCH] FEATURE: make date pickers in the bookmarks UI and topic timer UI consistent with all other pickers (#13365) Next Week should mean next Monday, Next Month - the first day of the next month, and so on. Also, we'll be using the name "Next Monday" instead of "Next Week" because it's easier to understand. No one can get confused by next Monday. --- .../app/components/edit-topic-timer-form.js | 19 +++--- .../app/components/time-shortcut-picker.js | 4 -- .../controllers/keyboard-shortcuts-help.js | 4 -- .../discourse/app/lib/time-shortcut.js | 9 --- .../discourse/app/lib/time-utils.js | 8 +-- .../modal/keyboard-shortcuts-help.hbs | 1 - .../tests/acceptance/bookmarks-test.js | 4 -- .../tests/acceptance/topic-edit-timer-test.js | 58 +++++++++++++++--- .../integration/components/bookmark-test.js | 59 ++++++++++++++++++- .../tests/unit/lib/time-utils-test.js | 9 +-- config/locales/client.en.yml | 4 +- 11 files changed, 124 insertions(+), 55 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/edit-topic-timer-form.js b/app/assets/javascripts/discourse/app/components/edit-topic-timer-form.js index cb3ad395236..65ab8b6b92c 100644 --- a/app/assets/javascripts/discourse/app/components/edit-topic-timer-form.js +++ b/app/assets/javascripts/discourse/app/components/edit-topic-timer-form.js @@ -14,7 +14,12 @@ import I18n from "I18n"; import { action } from "@ember/object"; import Component from "@ember/component"; import { isEmpty } from "@ember/utils"; -import { now, startOfDay, thisWeekend } from "discourse/lib/time-utils"; +import { + MOMENT_MONDAY, + now, + startOfDay, + thisWeekend, +} from "discourse/lib/time-utils"; import KeyboardShortcuts from "discourse/lib/keyboard-shortcuts"; import Mousetrap from "mousetrap"; @@ -82,22 +87,22 @@ export default Component.extend({ { icon: "bed", id: "this_weekend", - label: "topic.auto_update_input.this_weekend", + label: "time_shortcut.this_weekend", time: thisWeekend(), timeFormatKey: "dates.time_short_day", }, { icon: "far-clock", id: "two_weeks", - label: "topic.auto_update_input.two_weeks", - time: startOfDay(now().add(2, "weeks")), + label: "time_shortcut.two_weeks", + time: startOfDay(now().add(2, "weeks").day(MOMENT_MONDAY)), timeFormatKey: "dates.long_no_year", }, { icon: "far-calendar-plus", id: "six_months", - label: "topic.auto_update_input.six_months", - time: startOfDay(now().add(6, "months")), + label: "time_shortcut.six_months", + time: startOfDay(now().add(6, "months").startOf("month")), timeFormatKey: "dates.long_no_year", }, ]; @@ -105,7 +110,7 @@ export default Component.extend({ @discourseComputed hiddenTimeShortcutOptions() { - return ["none", "start_of_next_business_week"]; + return ["none"]; }, isCustom: equal("timerType", "custom"), 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 69d4a436d5b..7733cbab85c 100644 --- a/app/assets/javascripts/discourse/app/components/time-shortcut-picker.js +++ b/app/assets/javascripts/discourse/app/components/time-shortcut-picker.js @@ -32,10 +32,6 @@ const BINDINGS = { handler: "selectShortcut", args: [TIME_SHORTCUT_TYPES.TOMORROW], }, - "n w": { - handler: "selectShortcut", - args: [TIME_SHORTCUT_TYPES.NEXT_WEEK], - }, "n b w": { handler: "selectShortcut", args: [TIME_SHORTCUT_TYPES.START_OF_NEXT_BUSINESS_WEEK], diff --git a/app/assets/javascripts/discourse/app/controllers/keyboard-shortcuts-help.js b/app/assets/javascripts/discourse/app/controllers/keyboard-shortcuts-help.js index f717a0fe795..eaf65105b3f 100644 --- a/app/assets/javascripts/discourse/app/controllers/keyboard-shortcuts-help.js +++ b/app/assets/javascripts/discourse/app/controllers/keyboard-shortcuts-help.js @@ -151,10 +151,6 @@ export default Controller.extend(ModalFunctionality, { keys1: ["n", "d"], shortcutsDelimiter: "space", }), - next_week: buildShortcut("bookmarks.next_week", { - keys1: ["n", "w"], - shortcutsDelimiter: "space", - }), next_business_week: buildShortcut("bookmarks.next_business_week", { keys1: ["n", "b", "w"], shortcutsDelimiter: "space", diff --git a/app/assets/javascripts/discourse/app/lib/time-shortcut.js b/app/assets/javascripts/discourse/app/lib/time-shortcut.js index ddb4af44937..d2eac0b9c7a 100644 --- a/app/assets/javascripts/discourse/app/lib/time-shortcut.js +++ b/app/assets/javascripts/discourse/app/lib/time-shortcut.js @@ -4,7 +4,6 @@ import { laterToday, nextBusinessWeekStart, nextMonth, - nextWeek, now, tomorrow, } from "discourse/lib/time-utils"; @@ -13,7 +12,6 @@ import I18n from "I18n"; export const TIME_SHORTCUT_TYPES = { LATER_TODAY: "later_today", TOMORROW: "tomorrow", - NEXT_WEEK: "next_week", NEXT_MONTH: "next_month", CUSTOM: "custom", RELATIVE: "relative", @@ -63,13 +61,6 @@ export function defaultShortcutOptions(timezone) { I18n.t("dates.long_no_year") ), }, - { - icon: "far-clock", - id: TIME_SHORTCUT_TYPES.NEXT_WEEK, - label: "time_shortcut.next_week", - time: nextWeek(timezone), - timeFormatted: nextWeek(timezone).format(I18n.t("dates.long_no_year")), - }, { icon: "far-calendar-plus", id: TIME_SHORTCUT_TYPES.NEXT_MONTH, diff --git a/app/assets/javascripts/discourse/app/lib/time-utils.js b/app/assets/javascripts/discourse/app/lib/time-utils.js index 25976725cb6..1f765ffdc0d 100644 --- a/app/assets/javascripts/discourse/app/lib/time-utils.js +++ b/app/assets/javascripts/discourse/app/lib/time-utils.js @@ -37,16 +37,12 @@ export function laterThisWeek(timezone) { return startOfDay(now(timezone).add(2, "days")); } -export function nextWeek(timezone) { - return startOfDay(now(timezone).add(7, "days")); -} - export function nextMonth(timezone) { - return startOfDay(now(timezone).add(1, "month")); + return startOfDay(now(timezone).add(1, "month").startOf("month")); } export function nextBusinessWeekStart(timezone) { - return nextWeek(timezone).day(MOMENT_MONDAY); + return startOfDay(now(timezone).add(7, "days")).day(MOMENT_MONDAY); } export function parseCustomDatetime( diff --git a/app/assets/javascripts/discourse/app/templates/modal/keyboard-shortcuts-help.hbs b/app/assets/javascripts/discourse/app/templates/modal/keyboard-shortcuts-help.hbs index 514080379da..9d3b182ebc5 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/keyboard-shortcuts-help.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/keyboard-shortcuts-help.hbs @@ -64,7 +64,6 @@
  • {{html-safe shortcuts.bookmarks.later_today}}
  • {{html-safe shortcuts.bookmarks.later_this_week}}
  • {{html-safe shortcuts.bookmarks.tomorrow}}
    • -
  • {{html-safe shortcuts.bookmarks.next_week}}
  • {{html-safe shortcuts.bookmarks.next_month}}
  • {{html-safe shortcuts.bookmarks.next_business_week}}
  • {{html-safe shortcuts.bookmarks.next_business_day}}
    • diff --git a/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js b/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js index daf77fffdea..bab0ff76132 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js @@ -80,9 +80,6 @@ acceptance("Bookmarking", function (needs) { await openBookmarkModal(); await click("#tap_tile_start_of_next_business_week"); - await openBookmarkModal(); - await click("#tap_tile_next_week"); - await openBookmarkModal(); await click("#tap_tile_next_month"); @@ -96,7 +93,6 @@ acceptance("Bookmarking", function (needs) { assert.deepEqual(steps, [ "tomorrow", "start_of_next_business_week", - "next_week", "next_month", "custom", ]); diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-edit-timer-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-edit-timer-test.js index 0e3a35acb06..b06464376ef 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-edit-timer-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/topic-edit-timer-test.js @@ -1,14 +1,18 @@ import { acceptance, exists, + fakeTime, + loggedInUser, queryAll, updateCurrentUser, } from "discourse/tests/helpers/qunit-helpers"; import { click, fillIn, visit } from "@ember/test-helpers"; import { test } from "qunit"; import selectKit from "discourse/tests/helpers/select-kit-helper"; +import I18n from "I18n"; acceptance("Topic - Edit timer", function (needs) { + let clock = null; needs.user(); needs.pretender((server, helper) => { server.post("/t/280/timer", () => @@ -25,13 +29,23 @@ acceptance("Topic - Edit timer", function (needs) { ); }); + needs.hooks.beforeEach(() => { + const timezone = loggedInUser().resolvedTimezone(loggedInUser()); + const tuesday = "2100-06-15T08:00:00"; + clock = fakeTime(tuesday, timezone, true); + }); + + needs.hooks.afterEach(() => { + clock.restore(); + }); + test("autoclose - specific time", async function (assert) { updateCurrentUser({ moderator: true }); await visit("/t/internationalization-localization"); await click(".toggle-admin-menu"); await click(".admin-topic-timer-update button"); - await click("#tap_tile_next_week"); + await click("#tap_tile_start_of_next_business_week"); const regex = /will automatically close in/g; const html = queryAll(".edit-topic-timer-modal .topic-timer-info") @@ -47,7 +61,7 @@ acceptance("Topic - Edit timer", function (needs) { await click(".toggle-admin-menu"); await click(".admin-topic-timer-update button"); - await click("#tap_tile_next_week"); + await click("#tap_tile_start_of_next_business_week"); const regex1 = /will automatically close in/g; const html1 = queryAll(".edit-topic-timer-modal .topic-timer-info") @@ -56,7 +70,7 @@ acceptance("Topic - Edit timer", function (needs) { assert.ok(regex1.test(html1)); await click("#tap_tile_custom"); - await fillIn(".tap-tile-date-input .date-picker", "2099-11-24"); + await fillIn(".tap-tile-date-input .date-picker", "2100-11-24"); const regex2 = /will automatically close in/g; const html2 = queryAll(".edit-topic-timer-modal .topic-timer-info") @@ -89,7 +103,7 @@ acceptance("Topic - Edit timer", function (needs) { await timerType.expand(); await timerType.selectRowByValue("open"); - await click("#tap_tile_next_week"); + await click("#tap_tile_start_of_next_business_week"); const regex1 = /will automatically open in/g; const html1 = queryAll(".edit-topic-timer-modal .topic-timer-info") @@ -98,7 +112,7 @@ acceptance("Topic - Edit timer", function (needs) { assert.ok(regex1.test(html1)); await click("#tap_tile_custom"); - await fillIn(".tap-tile-date-input .date-picker", "2099-11-24"); + await fillIn(".tap-tile-date-input .date-picker", "2100-11-24"); const regex2 = /will automatically open in/g; const html2 = queryAll(".edit-topic-timer-modal .topic-timer-info") @@ -125,7 +139,7 @@ acceptance("Topic - Edit timer", function (needs) { await categoryChooser.expand(); await categoryChooser.selectRowByValue("7"); - await click("#tap_tile_next_week"); + await click("#tap_tile_start_of_next_business_week"); const regex = /will be published to #dev/g; const text = queryAll(".edit-topic-timer-modal .topic-timer-info") @@ -153,7 +167,7 @@ acceptance("Topic - Edit timer", function (needs) { ); await click("#tap_tile_custom"); - await fillIn(".tap-tile-date-input .date-picker", "2099-11-24"); + await fillIn(".tap-tile-date-input .date-picker", "2100-11-24"); await fillIn("#custom-time", "10:30"); await click(".edit-topic-timer-buttons button.btn-primary"); @@ -164,7 +178,7 @@ acceptance("Topic - Edit timer", function (needs) { exists("#tap_tile_last_custom"), "it show last custom because the custom date and time was valid" ); - let text = queryAll("#tap_tile_last_custom").text().trim(); + const text = queryAll("#tap_tile_last_custom").text().trim(); const regex = /Nov 24, 10:30 am/g; assert.ok(regex.test(text)); }); @@ -209,7 +223,7 @@ acceptance("Topic - Edit timer", function (needs) { await visit("/t/internationalization-localization"); await click(".toggle-admin-menu"); await click(".admin-topic-timer-update button"); - await click("#tap_tile_next_week"); + await click("#tap_tile_start_of_next_business_week"); await click(".edit-topic-timer-buttons button.btn-primary"); const removeTimerButton = queryAll(".topic-timer-info .topic-timer-remove"); @@ -219,4 +233,30 @@ acceptance("Topic - Edit timer", function (needs) { const topicTimerInfo = queryAll(".topic-timer-info .topic-timer-remove"); assert.equal(topicTimerInfo.length, 0); }); + + test("Shows correct time frame options", async function (assert) { + updateCurrentUser({ moderator: true }); + + await visit("/t/internationalization-localization"); + await click(".toggle-admin-menu"); + await click(".admin-topic-timer-update button"); + + const expected = [ + I18n.t("time_shortcut.tomorrow"), + I18n.t("time_shortcut.this_weekend"), + I18n.t("time_shortcut.start_of_next_business_week"), + I18n.t("time_shortcut.two_weeks"), + I18n.t("time_shortcut.next_month"), + I18n.t("time_shortcut.six_months"), + I18n.t("time_shortcut.custom"), + ]; + + const options = Array.from( + queryAll("div.tap-tile-grid div.tap-tile-title").map((_, div) => + div.innerText.trim() + ) + ); + + assert.deepEqual(options, expected); + }); }); diff --git a/app/assets/javascripts/discourse/tests/integration/components/bookmark-test.js b/app/assets/javascripts/discourse/tests/integration/components/bookmark-test.js index ba6d71d3435..4a637123c6b 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/bookmark-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/bookmark-test.js @@ -5,13 +5,20 @@ import { discourseModule, fakeTime, query, + queryAll, } from "discourse/tests/helpers/qunit-helpers"; +import I18n from "I18n"; discourseModule("Integration | Component | bookmark", function (hooks) { setupRenderingTest(hooks); - let template = - '{{bookmark model=model afterSave=afterSave afterDelete=afterDelete onCloseWithoutSaving=onCloseWithoutSaving registerOnCloseHandler=(action "registerOnCloseHandler") closeModal=(action "closeModal")}}'; + const template = `{{bookmark + model=model + afterSave=afterSave + afterDelete=afterDelete + onCloseWithoutSaving=onCloseWithoutSaving + registerOnCloseHandler=(action "registerOnCloseHandler") + closeModal=(action "closeModal")}}`; hooks.beforeEach(function () { this.actions.registerOnCloseHandler = () => {}; @@ -30,6 +37,35 @@ discourseModule("Integration | Component | bookmark", function (hooks) { } }); + componentTest("shows correct options", { + template, + + beforeEach() { + const tuesday = "2100-06-08T08:00:00"; + this.clock = fakeTime(tuesday, this.currentUser._timezone, true); + }, + + async test(assert) { + const expected = [ + I18n.t("time_shortcut.later_today"), + I18n.t("time_shortcut.tomorrow"), + I18n.t("time_shortcut.later_this_week"), + I18n.t("time_shortcut.start_of_next_business_week"), + I18n.t("time_shortcut.next_month"), + I18n.t("time_shortcut.custom"), + I18n.t("time_shortcut.none"), + ]; + + const options = Array.from( + queryAll( + "div.control-group div.tap-tile-grid div.tap-tile-title" + ).map((_, div) => div.innerText.trim()) + ); + + assert.deepEqual(options, expected); + }, + }); + componentTest("show later this week option if today is < Thursday", { template, @@ -156,4 +192,23 @@ discourseModule("Integration | Component | bookmark", function (hooks) { assert.equal(query("#custom-time").value, "08:00"); }, }); + + componentTest("Next Month points to the first day of the next month", { + template, + + beforeEach() { + this.clock = fakeTime( + "2100-01-01T08:00:00", + this.currentUser._timezone, + true + ); + }, + + async test(assert) { + assert.equal( + query("div#tap_tile_next_month div.tap-tile-date").innerText, + "Feb 1, 8:00 am" + ); + }, + }); }); diff --git a/app/assets/javascripts/discourse/tests/unit/lib/time-utils-test.js b/app/assets/javascripts/discourse/tests/unit/lib/time-utils-test.js index 6cfca1c0439..5b84924527c 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/time-utils-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/time-utils-test.js @@ -7,7 +7,6 @@ import { laterThisWeek, laterToday, nextMonth, - nextWeek, startOfDay, tomorrow, } from "discourse/lib/time-utils"; @@ -16,15 +15,9 @@ import { test } from "qunit"; const timezone = "Australia/Brisbane"; discourseModule("Unit | lib | timeUtils", function () { - test("nextWeek gets next week correctly", function (assert) { - withFrozenTime("2019-12-11T08:00:00", timezone, () => { - assert.equal(nextWeek(timezone).format("YYYY-MM-DD"), "2019-12-18"); - }); - }); - test("nextMonth gets next month correctly", function (assert) { withFrozenTime("2019-12-11T08:00:00", timezone, () => { - assert.equal(nextMonth(timezone).format("YYYY-MM-DD"), "2020-01-11"); + assert.equal(nextMonth(timezone).format("YYYY-MM-DD"), "2020-01-01"); }); }); diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6fc103c8862..6eda56e15b6 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -596,12 +596,14 @@ en: later_today: "Later today" next_business_day: "Next business day" tomorrow: "Tomorrow" - next_week: "Next week" post_local_date: "Date in post" later_this_week: "Later this week" + this_weekend: "This weekend" start_of_next_business_week: "Monday" start_of_next_business_week_alt: "Next Monday" + two_weeks: "Two weeks" next_month: "Next month" + six_months: "Six months" custom: "Custom date and time" relative: "Relative time" none: "None needed"