From fdcb42914580f274bcb159a6b2f78a0a9e84e340 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Tue, 7 Mar 2023 11:05:13 +0800 Subject: [PATCH] FIX: Handle null values in category settings relative time pickers (#20552) As reported on Meta, the relative time pickers for configuring slow-mode and auto-close durations in category settings are initially showing a "mins" option, which then disappears after you select any other timescale. Our `RelativeTimePicker` component wasn't equipped to handle `null` values as the initial input. This caused it to go into a code path that set the selected timescale to "mins", even if that is not an allowed option. There are two things being done here: 1. Add support for `null` input values to `RelativeTimePicker`. This fixes the auto-close setting. 2. Allow minutes for the slow-mode setting. (The user in Meta mentioned they usually set 15-30 minutes to cool down hot topics. --- .../app/components/edit-category-settings.hbs | 1 - .../app/components/relative-time-picker.js | 19 ++++++++++++------- .../components/relative-time-picker-test.js | 16 ++++++++++++++++ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/edit-category-settings.hbs b/app/assets/javascripts/discourse/app/components/edit-category-settings.hbs index 27fe8267c80..647ce9c3fd5 100644 --- a/app/assets/javascripts/discourse/app/components/edit-category-settings.hbs +++ b/app/assets/javascripts/discourse/app/components/edit-category-settings.hbs @@ -153,7 +153,6 @@ diff --git a/app/assets/javascripts/discourse/app/components/relative-time-picker.js b/app/assets/javascripts/discourse/app/components/relative-time-picker.js index cb23e50cadf..29e91b71e25 100644 --- a/app/assets/javascripts/discourse/app/components/relative-time-picker.js +++ b/app/assets/javascripts/discourse/app/components/relative-time-picker.js @@ -14,19 +14,19 @@ export default Component.extend({ @on("init") cloneDuration() { - let mins = this.durationMinutes; - let hours = this.durationHours; + const usesHours = Object.hasOwn(this.attrs, "durationHours"); + const usesMinutes = Object.hasOwn(this.attrs, "durationMinutes"); - if (hours && mins) { + if (usesHours && usesMinutes) { throw new Error( "relative-time needs initial duration in hours OR minutes, both are not supported" ); } - if (hours) { - this._setInitialDurationFromHours(hours); + if (usesHours) { + this._setInitialDurationFromHours(this.durationHours); } else { - this._setInitialDurationFromMinutes(mins); + this._setInitialDurationFromMinutes(this.durationMinutes); } }, @@ -47,7 +47,12 @@ export default Component.extend({ }, _setInitialDurationFromHours(hours) { - if (hours >= 8760) { + if (hours === null) { + this.setProperties({ + duration: hours, + selectedInterval: "hours", + }); + } else if (hours >= 8760) { this.setProperties({ duration: this._roundedDuration(hours / 365 / 24), selectedInterval: "years", diff --git a/app/assets/javascripts/discourse/tests/integration/components/relative-time-picker-test.js b/app/assets/javascripts/discourse/tests/integration/components/relative-time-picker-test.js index 5cc16d0ec17..f34169e9d84 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/relative-time-picker-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/relative-time-picker-test.js @@ -20,6 +20,14 @@ module("Integration | Component | relative-time-picker", function (hooks) { assert.strictEqual(prefilledDuration, "5"); }); + test("prefills and preselects null minutes", async function (assert) { + await render(hbs``); + + const prefilledDuration = query(".relative-time-duration").value; + assert.strictEqual(this.subject.header().value(), "mins"); + assert.strictEqual(prefilledDuration, ""); + }); + test("prefills and preselects hours based on translated minutes", async function (assert) { await render(hbs``); @@ -60,6 +68,14 @@ module("Integration | Component | relative-time-picker", function (hooks) { assert.strictEqual(prefilledDuration, "5"); }); + test("prefills and preselects null hours", async function (assert) { + await render(hbs``); + + const prefilledDuration = query(".relative-time-duration").value; + assert.strictEqual(this.subject.header().value(), "hours"); + assert.strictEqual(prefilledDuration, ""); + }); + test("prefills and preselects minutes based on translated hours", async function (assert) { await render(hbs``);