Modify datetime picker for scheduling articles (forem#18153)

* Accept separate date and time for published_at * Separate date and time inputs for published_at in post options (scheduling) * Removed the commented line * Removed unused htmlFor publishedAt Co-authored-by: Suzanne Aitchison <[email protected]> * Added aria-label for published at date Co-authored-by: Suzanne Aitchison <[email protected]> * Added aria-label for published at time Co-authored-by: Suzanne Aitchison <[email protected]> * Added htmlFor for schedule label Co-authored-by: Suzanne Aitchison <[email protected]>
AlicanAkyol · Jul 21, 2022 · 656d698 · 656d698
1 parent 0e208a6
commit 656d698
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 45 deletions.
diff --git a/app/controllers/articles_controller.rb b/app/controllers/articles_controller.rb
@@ -306,6 +306,7 @@ def article_params_json
                        %i[
                          title body_markdown main_image published description video_thumbnail_url
                          tag_list canonical_url series collection_id archived published_at timezone
+                         published_at_date published_at_time
                        ]
                      end
 
@@ -319,10 +320,14 @@ def article_params_json
     end
 
     time_zone_str = params["article"].delete("timezone")
-    if params["article"]["published_at"]
+
+    time = params["article"].delete("published_at_time")
+    date = params["article"].delete("published_at_date")
+
+    if date.present?
       time_zone = Time.find_zone(time_zone_str)
       time_zone ||= Time.find_zone("UTC")
-      params["article"]["published_at"] = time_zone.parse(params["article"]["published_at"])
+      params["article"]["published_at"] = time_zone.parse("#{date} #{time}")
     end
 
     @article_params_json = params.require(:article).permit(allowed_params)

diff --git a/app/javascript/article-form/articleForm.jsx b/app/javascript/article-form/articleForm.jsx
@@ -2,6 +2,7 @@ import { h, Component } from 'preact';
 import PropTypes from 'prop-types';
 import linkState from 'linkstate';
 import postscribe from 'postscribe';
+import moment from 'moment';
 import { KeyboardShortcuts } from '../shared/components/useKeyboardShortcuts';
 import { embedGists } from '../utilities/gist';
 import { submitArticle, previewArticle } from './actions';
@@ -18,7 +19,7 @@ import { getOSKeyboardModifierKeyString } from '@utilities/runtime';
 /* global activateRunkitTags */
 
 /*
-  Although the state fields: id, description, canonicalUrl, publishedAt, series, allSeries and
+  Although the state fields: id, description, canonicalUrl, publishedAtDate, publishedAtTime, series, allSeries and
   editing are not used in this file, they are important to the
   editor.
 */
@@ -96,14 +97,23 @@ export class ArticleForm extends Component {
           }
         : {};
 
+    this.publishedAtTime, (this.publishedAtDate = '');
+
+    if (this.article.published_at) {
+      const publishedAt = moment(this.article.published_at);
+      this.publishedAtTime = publishedAt.format('HH:mm');
+      this.publishedAtDate = publishedAt.format('YYYY-MM-DD');
+    }
+
     this.state = {
       formKey: new Date().toISOString(),
       id: this.article.id || null, // eslint-disable-line react/no-unused-state
       title: this.article.title || '',
       tagList: this.article.cached_tag_list || '',
       description: '', // eslint-disable-line react/no-unused-state
       canonicalUrl: this.article.canonical_url || '', // eslint-disable-line react/no-unused-state
-      publishedAt: this.article.published_at || '', // eslint-disable-line react/no-unused-state
+      publishedAtTime: this.publishedAtTime,
+      publishedAtDate: this.publishedAtDate,
       timezone: Intl.DateTimeFormat().resolvedOptions().timeZone || '', // eslint-disable-line react/no-unused-state
       series: this.article.series || '', // eslint-disable-line react/no-unused-state
       allSeries: this.article.all_series || [], // eslint-disable-line react/no-unused-state
@@ -335,7 +345,8 @@ export class ArticleForm extends Component {
       tagList: this.article.cached_tag_list || '',
       description: '', // eslint-disable-line react/no-unused-state
       canonicalUrl: this.article.canonical_url || '', // eslint-disable-line react/no-unused-state
-      publishedAt: this.article.published_at || '', // eslint-disable-line react/no-unused-state
+      publishedAtTime: this.publishedAtTime,
+      publishedAtDate: this.publishedAtDate,
       series: this.article.series || '', // eslint-disable-line react/no-unused-state
       allSeries: this.article.all_series || [], // eslint-disable-line react/no-unused-state
       bodyMarkdown: this.article.body_markdown || '',
@@ -397,7 +408,8 @@ export class ArticleForm extends Component {
       tagList,
       bodyMarkdown,
       published,
-      publishedAt,
+      publishedAtTime,
+      publishedAtDate,
       previewShowing,
       previewLoading,
       previewResponse,
@@ -498,7 +510,8 @@ export class ArticleForm extends Component {
 
         <EditorActions
           published={published}
-          publishedAt={publishedAt}
+          publishedAtTime={publishedAtTime}
+          publishedAtDate={publishedAtDate}
           schedulingEnabled={schedulingEnabled}
           version={version}
           onPublish={this.onPublish}

diff --git a/app/javascript/article-form/components/EditorActions.jsx b/app/javascript/article-form/components/EditorActions.jsx
@@ -1,4 +1,5 @@
 import { h } from 'preact';
+import moment from 'moment';
 import PropTypes from 'prop-types';
 import { Options } from './Options';
 import { ButtonNew as Button } from '@crayons';
@@ -8,7 +9,8 @@ export const EditorActions = ({
   onPublish,
   onClearChanges,
   published,
-  publishedAt,
+  publishedAtDate,
+  publishedAtTime,
   schedulingEnabled,
   edited,
   version,
@@ -37,9 +39,11 @@ export const EditorActions = ({
     );
   }
 
-  const now = new Date();
-  const publishedAtDate = publishedAt ? new Date(publishedAt) : now;
-  const schedule = publishedAtDate > now;
+  const now = moment();
+  const publishedAtObj = publishedAtDate
+    ? moment(`${publishedAtDate} ${publishedAtTime || '00:00'}`)
+    : now;
+  const schedule = publishedAtObj > now;
 
   const saveButtonText = schedule
     ? 'Schedule'
@@ -95,7 +99,8 @@ EditorActions.propTypes = {
   onSaveDraft: PropTypes.func.isRequired,
   onPublish: PropTypes.func.isRequired,
   published: PropTypes.bool.isRequired,
-  publishedAt: PropTypes.string.isRequired,
+  publishedAtTime: PropTypes.string.isRequired,
+  publishedAtDate: PropTypes.string.isRequired,
   schedulingEnabled: PropTypes.bool.isRequired,
   edited: PropTypes.bool.isRequired,
   version: PropTypes.string.isRequired,

diff --git a/app/javascript/article-form/components/Options.jsx b/app/javascript/article-form/components/Options.jsx
@@ -1,5 +1,6 @@
 import { h } from 'preact';
 import PropTypes from 'prop-types';
+import moment from 'moment';
 import { Dropdown, ButtonNew as Button } from '@crayons';
 import CogIcon from '@images/cog.svg';
 
@@ -12,24 +13,11 @@ import CogIcon from '@images/cog.svg';
  * @param {Function} props.onConfigChange Callback for when the config options have changed
  */
 
-function toISOStringLocal(datetime) {
-  const month = (datetime.getMonth() + 1).toString().padStart(2, '0');
-  const day = datetime.getDate().toString().padStart(2, '0');
-  return [
-    datetime.getFullYear(),
-    '-',
-    month,
-    '-',
-    day,
-    'T',
-    datetime.toLocaleTimeString(),
-  ].join('');
-}
-
 export const Options = ({
   passedData: {
     published = false,
-    publishedAt = '',
+    publishedAtDate = '',
+    publishedAtTime = '',
     timezone = Intl.DateTimeFormat().resolvedOptions().timeZone,
     allSeries = [],
     canonicalUrl = '',
@@ -43,13 +31,9 @@ export const Options = ({
   let publishedField = '';
   let existingSeries = '';
   let publishedAtField = '';
-  const publishedDate = new Date(publishedAt);
-  const currentDate = new Date();
-
-  const localPublishedAt = toISOStringLocal(publishedDate);
-  const minPublishedAt = toISOStringLocal(currentDate);
 
-  const readonlyPublishedAt = published && publishedDate < currentDate;
+  const readonlyPublishedAt =
+    published && moment(`${publishedAtDate} ${publishedAtTime}`) < moment();
 
   if (allSeries.length > 0) {
     const seriesNames = allSeries.map((name, index) => {
@@ -92,20 +76,33 @@ export const Options = ({
       </div>
     );
   }
+
   if (schedulingEnabled && !readonlyPublishedAt) {
+    const currentDate = moment().format('YYYY-MM-DD');
     publishedAtField = (
       <div className="crayons-field mb-6">
-        <label htmlFor="publishedAt" className="crayons-field__label">
+        <label htmlFor="publishedAtDate" className="crayons-field__label">
           Schedule Publication
         </label>
         <input
-          type="datetime-local"
-          min={minPublishedAt}
-          value={localPublishedAt} // "2022-04-28T15:00:00"
+          aria-label="Schedule publication date"
+          type="date"
+          min={currentDate}
+          value={publishedAtDate} // ""
+          className="crayons-textfield"
+          name="publishedAtDate"
+          onChange={onConfigChange}
+          id="publishedAtDate"
+          placeholder="..."
+        />
+        <input
+          aria-label="Schedule publication time"
+          type="time"
+          value={publishedAtTime} // "18:00"
           className="crayons-textfield"
-          name="publishedAt"
+          name="publishedAtTime"
           onChange={onConfigChange}
-          id="publishedAt"
+          id="publishedAtTime"
           placeholder="..."
         />
         <input
@@ -195,7 +192,8 @@ export const Options = ({
 Options.propTypes = {
   passedData: PropTypes.shape({
     published: PropTypes.bool.isRequired,
-    publishedAt: PropTypes.string.isRequired,
+    publishedAtDate: PropTypes.string.isRequired,
+    publishedAtTime: PropTypes.string.isRequired,
     timezone: PropTypes.string.isRequired,
     allSeries: PropTypes.array.isRequired,
     canonicalUrl: PropTypes.string.isRequired,

diff --git a/spec/requests/articles/articles_create_spec.rb b/spec/requests/articles/articles_create_spec.rb
@@ -118,10 +118,12 @@
     let(:tomorrow) { 1.day.from_now }
     let(:attributes) do
       { title: new_title, body_markdown: "Yo ho ho#{rand(100)}",
-        published_at: "#{tomorrow.strftime('%Y-%m-%d')} 18:00" }
+        published: true,
+        published_at_date: tomorrow.strftime("%Y-%m-%d"),
+        published_at_time: "18:00" }
     end
 
-    it "sets published_at according to the timezone" do
+    it "sets published_at according to the timezone new" do
       attributes[:timezone] = "Europe/Moscow"
       post "/articles", params: { article: attributes }
       a = Article.find_by(title: new_title)
@@ -130,13 +132,32 @@
     end
 
     # crossing the date line
-    it "sets published_at for another timezone" do
+    it "sets published_at for another timezone new" do
       attributes[:timezone] = "Pacific/Honolulu"
       post "/articles", params: { article: attributes }
       a = Article.find_by(title: new_title)
       published_at_utc = a.published_at.in_time_zone("UTC").strftime("%m/%d/%Y %H:%M")
       expect(published_at_utc).to eq("#{(tomorrow + 1.day).strftime('%m/%d/%Y')} 04:00")
     end
+
+    it "sets published_at when only date is passed" do
+      attributes[:published_at_date] = 2.days.from_now.strftime("%Y-%m-%d")
+      attributes[:published_at_time] = nil
+      attributes[:timezone] = "Europe/Moscow"
+      post "/articles", params: { article: attributes }
+      a = Article.find_by(title: new_title)
+      # 00:00 in user timezone (attributes[:timezone])
+      published_at_utc = a.published_at.in_time_zone("UTC").strftime("%m/%d/%Y %H:%M")
+      expect(published_at_utc).to eq("#{tomorrow.strftime('%m/%d/%Y')} 21:00")
+    end
+
+    it "sets current published_at when only time is passed" do
+      attributes[:published_at_date] = nil
+      attributes[:timezone] = "Asia/Magadan"
+      post "/articles", params: { article: attributes }
+      a = Article.find_by(title: new_title)
+      expect(a.published_at).to be_within(1.minute).of(Time.current)
+    end
   end
 
   context "when setting published_at from editor v1" do

diff --git a/spec/requests/articles/articles_update_spec.rb b/spec/requests/articles/articles_update_spec.rb
@@ -162,14 +162,18 @@
   context "when setting published_at in editor v2" do
     let(:tomorrow) { 1.day.from_now }
     let(:published_at) { "#{tomorrow.strftime('%d.%m.%Y')} 18:00" }
+    let(:published_at_date) { tomorrow.strftime("%d.%m.%Y") }
+    let(:published_at_time) { "18:00" }
     let(:attributes) do
-      { title: "NEW TITLE #{rand(100)}", body_markdown: "Yo ho ho#{rand(100)}", published_at: published_at }
+      { title: "NEW TITLE #{rand(100)}", body_markdown: "Yo ho ho#{rand(100)}",
+        published_at_date: published_at_date, published_at_time: published_at_time }
     end
 
     # scheduled => scheduled
     it "updates published_at from scheduled to scheduled" do
       article.update_column(:published_at, 3.days.from_now)
       attributes[:timezone] = "Europe/Moscow"
+      attributes[:published] = true
       put "/articles/#{article.id}", params: { article: attributes }
       article.reload
       published_at_utc = article.published_at.in_time_zone("UTC").strftime("%m/%d/%Y %H:%M")
@@ -180,7 +184,6 @@
     it "sets published_at according to the timezone when updating draft => scheduled" do
       draft = create(:article, published: false, user_id: user.id)
       attributes[:published] = true
-      attributes[:published_at] = "#{tomorrow.strftime('%d/%m/%Y')} 18:00"
       attributes[:timezone] = "America/Mexico_City"
       put "/articles/#{draft.id}", params: { article: attributes }
       draft.reload