From 64a96a1595ce65f02ab7513402c501acb9a94d9c Mon Sep 17 00:00:00 2001 From: Gregg8 Date: Wed, 3 Nov 2021 12:28:27 +1100 Subject: [PATCH] [daterange] Gregg's comments, changes and suggestions --- src/re_com/daterange.cljs | 138 +++++++++++++++++++++---------------- src/re_demo/daterange.cljs | 78 ++++++++++++--------- 2 files changed, 123 insertions(+), 93 deletions(-) diff --git a/src/re_com/daterange.cljs b/src/re_com/daterange.cljs index 10a594bb..e7dd9d78 100644 --- a/src/re_com/daterange.cljs +++ b/src/re_com/daterange.cljs @@ -1,16 +1,16 @@ (ns re-com.daterange (:require-macros - [re-com.core :refer [handler-fn at reflect-current-component]]) + [re-com.core :refer [handler-fn at reflect-current-component]]) ;; TODO [GR-REMOVE] I like to have requires lined up (not everyone cares) (:require - [reagent.core :as r] - [re-com.config :refer [include-args-desc?]] - [re-com.box :refer [line border flex-child-style]] - [re-com.core :as re-com :refer [at v-box h-box box popover-anchor-wrapper popover-content-wrapper]] - [re-com.validate :refer [date-like? css-style? html-attr? parts?] :refer-macros [validate-args-macro]] - [re-com.util :refer [deref-or-value now->utc]] - [cljs-time.format :refer [parse unparse formatter]] - [cljs-time.core :as cljs-time] - [goog.string :as gstring :refer [format]]) + [reagent.core :as r] + [re-com.config :refer [include-args-desc?]] + [re-com.box :refer [line border flex-child-style]] + [re-com.core :refer [at v-box h-box box popover-anchor-wrapper popover-content-wrapper]] ;; TODO [GR-REMOVE] Removed unused `:as re-com` (also we normally use `:as rc`) + [re-com.validate :refer [date-like? css-style? html-attr? parts?] :refer-macros [validate-args-macro]] + [re-com.util :refer [deref-or-value now->utc]] + [cljs-time.format :refer [parse unparse formatter]] + [cljs-time.core :as cljs-time] + [goog.string :refer [format]]);; TODO [GR-REMOVE] Removed unused `:as gstring` (:import [goog.i18n DateTimeFormat])) @@ -172,7 +172,7 @@ [line] [next-year-nav current-month-atom parts]]]) -(defn- main-div-with +(defn- main-div-with ;; TODO [GR-REMOVE] Haven't updated it here but consider changing to kwargs so that calls are more self documenting "Main container to pass: class, style and attributes" [body hide-border? class style attr parts src debug-as] [h-box @@ -200,16 +200,16 @@ (defn- date-disabled? "Checks various things to see if a date had been disabled." [date [minimum maximum disabled? selectable-fn]] - (let [too-early? (when minimum (cljs-time/before? date (deref-or-value minimum))) - too-late? (when maximum (cljs-time/after? date (deref-or-value maximum))) + (let [too-early? (when minimum (cljs-time/before? date (deref-or-value minimum))) ;; TODO: [GR-REMOVE] It's nice to have formatted let block (IntelliJ has a keyboard shortcut to do this - Highlight the block and press Ctrl+Alt+L) + too-late? (when maximum (cljs-time/after? date (deref-or-value maximum))) de-selected? (when selectable-fn (not (selectable-fn date)))] (or too-early? too-late? de-selected? disabled?))) (defn- create-interval "inclusively creates a vector of date-formats from start to end." [start end] - (let [first (deref-or-value start) - last (deref-or-value end)] + (let [first (deref-or-value start) ;; TODO: [GR-REMOVE] Format let block + last (deref-or-value end)] (loop [cur first result []] (if (cljs-time/after? cur last) result @@ -230,7 +230,9 @@ (if (and (= @fsm "pick-end") ;if we're picking and end date (cljs-time/before? @start-date day) - (if check-interval? (interval-valid? start-date day disabled-data) true)) + (if check-interval? ;; TODO [GR-REMOVE] Multi-line formatting more readable, especially when else clause is insignificant compared to if clause + (interval-valid? start-date day disabled-data) + true)) (do (reset! fsm "pick-start") (reset! end-date day) ;update the internal end-date value @@ -260,16 +262,17 @@ "Create table data elements with reactive classes and on click/hover handlers" [day [fsm start-date end-date temp-end] {:keys [on-change disabled? selectable-fn minimum maximum show-today? check-interval? parts] :as args}] (let [disabled-data (vector minimum maximum disabled? selectable-fn)] - (if (= day "") [:td ""] - (let [correct-class (class-for-td day start-date end-date temp-end disabled? selectable-fn minimum maximum show-today?) - clickable? (not (date-disabled? day disabled-data))] - (into [:td] - (vector (merge {:class (str "rc-daterange-td-basic " correct-class (get-in parts [:date :class])) - :style (get-in parts [:date :style]) - :on-click #(when clickable? (td-click-handler day [fsm start-date end-date] on-change check-interval? disabled-data)) - :on-mouse-enter #(reset! temp-end day)} - (get-in parts [:date :attr])) - (str (cljs-time/day day)))))))) + (if (= day "") ;; TODO [GR-REMOVE] Formatting + [:td ""] + (let [correct-class (class-for-td day start-date end-date temp-end disabled? selectable-fn minimum maximum show-today?) + clickable? (not (date-disabled? day disabled-data))] + (into [:td] + (vector (merge {:class (str "rc-daterange-td-basic " correct-class (get-in parts [:date :class])) + :style (get-in parts [:date :style]) + :on-click #(when clickable? (td-click-handler day [fsm start-date end-date] on-change check-interval? disabled-data)) + :on-mouse-enter #(reset! temp-end day)} + (get-in parts [:date :attr])) + (str (cljs-time/day day)))))))) (defn week-td [week-number] [:td {:class (str "daterange-td-basic " "daterange-week")} week-number]) @@ -285,15 +288,15 @@ (for [day days-list] [create-day-td day atoms args])))) -(defn- parse-date-from-ints - "Given 3 ints, parse them as a useable date-format e.g. 11 2 2021" - [d m y] - (parse (formatter "ddMMYYYY") (str (format "%02d" d) (format "%02d" m) (str y)))) +;(defn- parse-date-from-ints ;; TODO: [GR-REMOVE] This already exists as cljs-time/date-time - REMOVE THIS +; "Given 3 ints, parse them as a useable date-format e.g. 11 2 2021" +; [d m y] +; (parse (formatter "ddMMYYYY") (str (format "%02d" d) (format "%02d" m) (str y)))) (defn- empty-days-count "Returns the number of empty date tiles at the start of the month based on the first day of the month and the chosen week start day, monday = 1 sunday = 7" [chosen start] - (let [chosen (if chosen chosen 1)] ;default week start of monday + (let [chosen (or chosen 1)] ;default week start of monday ;; TODO: Change from (if chosen chosen 1) (if (> chosen start) (- 7 (- chosen start)) (- start chosen)))) @@ -301,13 +304,25 @@ (defn- days-for-month "Produces a partitioned list of date-formats with all the days in the given month, with leading empty strings to align with the days of the week" [date-from-month start-of-week] - (let [month (cljs-time/month date-from-month) - year (cljs-time/year date-from-month) + (let [month (cljs-time/month date-from-month) ;; TODO: [GR-REMOVE] Format let block + year (cljs-time/year date-from-month) last-day-of-month (cljs-time/day (cljs-time/last-day-of-the-month date-from-month)) - first-day-val (cljs-time/day-of-week (cljs-time/first-day-of-the-month date-from-month)) ;; 1 = mon 7 = sun - day-ints (range 1 (inc last-day-of-month)) ;; e.g. (1 2 3 ... 31) - days (map #(parse-date-from-ints % month year) day-ints) ;; turn into real date-times - with-lead-emptys (flatten (cons (repeat (empty-days-count start-of-week first-day-val) "") days))] ;; for padding the table + first-day-val (cljs-time/day-of-week (cljs-time/first-day-of-the-month date-from-month)) ;; 1 = mon 7 = sun + day-ints (range 1 (inc last-day-of-month)) ;; e.g. (1 2 3 ... 31) + + ;days (map #(parse-date-from-ints % month year) day-ints) ;; turn into real date-times + ;; TODO [GR-REMOVE] parse-date-from-ints is already implemented in cljs-time lib + days (map #(cljs-time/date-time year month %) day-ints) ;; turn into real date-times + + ;with-lead-emptys (flatten (cons (repeat (empty-days-count start-of-week first-day-val) "") days)) ;; for padding the table + ;; TODO [GR-REMOVE] I have replaced the above line with a generally more readable (and more easily edited) version (will let you look for other examples to update + with-lead-emptys (-> start-of-week ;; for padding the table + (empty-days-count first-day-val) + (repeat "") + (cons days) + flatten) + + ] (partition-all 7 with-lead-emptys))) ;; split into lists of 7 to be passed to create-week-tr (def days-vec [[:td "M"] [:td "Tu"] [:td "W"] [:td "Th"] [:td "F"] [:td "Sa"] [:td "Su"]]) ;for cycling and display depending on start-of-week @@ -315,24 +330,25 @@ (defn- create-table "Given the result from days-for-month for a given month, create the :tbody using the relevant :tr and :td functions above" [date atoms {:keys [start-of-week i18n parts show-weeks?] :as args}] - (let [into-tr (if show-weeks? [:tr [:td]] [:tr]) - days-of-week (if (:days i18n) - (map (fn [new-day [td _]] [td new-day]) (:days i18n) days-vec) ;update days vec with the changed days - days-vec) - add-parts (fn [[td day-string]] - (vector td (merge {:class (str "daterange-day-title" (get-in parts [:day-title :class])) - :style (get-in parts [:day-title :style])} - (get-in parts [:day-title :attr])) - day-string)) - with-parts (map #(add-parts %) days-of-week) + (let [into-tr (if show-weeks? [:tr [:td]] [:tr]) ;; TODO: [GR-REMOVE] Format let block + days-of-week (if (:days i18n) + (map (fn [new-day [td _]] [td new-day]) (:days i18n) days-vec) ;update days vec with the changed days + days-vec) + add-parts (fn [[td day-string]] + (vector td + (merge {:class (str "daterange-day-title" (get-in parts [:day-title :class])) + :style (get-in parts [:day-title :style])} + (get-in parts [:day-title :attr])) + day-string)) + with-parts (map #(add-parts %) days-of-week) table-row-weekdays (into into-tr (take 7 (drop (dec start-of-week) (cycle with-parts)))) - partitioned-days (days-for-month date start-of-week) - date-rows (for [x partitioned-days] - [create-week-tr x atoms args]) + partitioned-days (days-for-month date start-of-week) + date-rows (for [x partitioned-days] + [create-week-tr x atoms args]) - with-weekdays-row (into [:tbody table-row-weekdays]) - with-dates (into with-weekdays-row date-rows)] + with-weekdays-row (into [:tbody table-row-weekdays]) + with-dates (into with-weekdays-row date-rows)] [:table (merge {:class (str "rc-daterange-table" (get-in parts [:table :class])) :style (get-in parts [:table :style])} @@ -413,21 +429,21 @@ [& {:keys [model initial-display] :as args}] (or (validate-args-macro daterange-args-desc args) - (let [current-month (r/atom (or (deref-or-value initial-display) (now->utc))) - fsm (r/atom "pick-start") - start-date (r/atom (:start (deref-or-value model))) - end-date (r/atom (:end (deref-or-value model))) - temp-end (r/atom (now->utc))] ;for :on-hover css functionality + (let [current-month (r/atom (or (deref-or-value initial-display) (now->utc))) ;; TODO: [GR-REMOVE] Format let block + fsm (r/atom "pick-start") + start-date (r/atom (:start (deref-or-value model))) + end-date (r/atom (:end (deref-or-value model))) + temp-end (r/atom (now->utc))] ;for :on-hover css functionality (fn render-fn [& {:keys [model hide-border? i18n class style attr parts src debug-as] :as args}] (or (validate-args-macro daterange-args-desc args) ;re validate args each time they change - (let [latest-external-model (deref-or-value model) + (let [latest-external-model (deref-or-value model) ;; TODO: [GR-REMOVE] Format let block internal-model-refernce {:start @start-date :end @end-date}] (when (and (model-changed? latest-external-model internal-model-refernce) (= @fsm "pick-start")) (reset! start-date (:start latest-external-model)) (reset! end-date (:end latest-external-model))) - [main-div-with + [main-div-with ;; TODO [GR-REMOVE] Haven't updated it here but consider changing to kwargs so that calls are more self documenting [h-box :src (at) :gap "60px" :padding "15px" @@ -450,14 +466,14 @@ (defn- anchor-button "Provide clickable field with current date label and dropdown button e.g. [ 2014 Sep 17 | # ]" [shown? model format goog? placeholder width disabled?] - (let [format-str (if format format "dd MMM, yyyy")] + (let [format-str (or format "dd MMM, yyyy")] ;; TODO [GR-REMOVE] Changed from (if format format "dd MMM, yyyy") [:div {:class "rc-daterange-dropdown-anchor input-group display-flex noselect" :style (flex-child-style "none") :on-click (handler-fn (when (not (deref-or-value disabled?)) (swap! shown? not)))} [h-box - :width (if width width "228px") + :width (or width "228px") ;; TODO [GR-REMOVE] Changed from (if width width "228px") :children [[box :size "auto" :class (str "form-control dropdown-button" (when (deref-or-value disabled?) " dropdown-button-disabled")) diff --git a/src/re_demo/daterange.cljs b/src/re_demo/daterange.cljs index c3e89c11..f41475f0 100644 --- a/src/re_demo/daterange.cljs +++ b/src/re_demo/daterange.cljs @@ -10,10 +10,10 @@ [cljs-time.format :refer [formatter unparse]] [re-com.core :refer [at h-box v-box box gap single-dropdown datepicker datepicker-dropdown checkbox label title p button md-icon-button checkbox]] [re-com.datepicker :refer [iso8601->date datepicker-parts-desc datepicker-dropdown-args-desc]] - [re-com.daterange :as daterange :refer [daterange daterange-args-desc daterange-parts-desc daterange-dropdown-args-desc daterange-dropdown]] + [re-com.daterange :refer [daterange daterange-args-desc daterange-parts-desc daterange-dropdown-args-desc daterange-dropdown]] ;; TODO [GR-REMOVE] Removed unused `:as daterange` [re-com.validate :refer [date-like?]] [re-com.util :refer [now->utc px]] - [cljs-time.core :as cljs-time] + [cljs-time.core :as cljs-time] [re-demo.utils :refer [panel-title title2 title3 parts-table args-table github-hyperlink status-text]]) (:import [goog.i18n DateTimeSymbols_pl])) @@ -29,28 +29,28 @@ (defn create-checkbox [atom day] [v-box - :align :center + :align :center :children [[box :style {:font-size "smaller"} :child day] [checkbox :model ((keyword day) @atom) :on-change #(swap! atom update-in [(keyword day)] not)]]]) (defn holder [] - (let [dropdown-model (reagent/atom nil) - model-atom (reagent/atom nil) - today-model (reagent/atom false) - disabled-model (reagent/atom false) - weeks-model (reagent/atom false) - interval-model (reagent/atom false) + (let [dropdown-model (reagent/atom nil) ;; TODO [GR-REMOVE] Align definitions (IntelliJ Ctrl+Alt+L, not sure what it is in VS Code) + model-atom (reagent/atom nil) + today-model (reagent/atom false) + disabled-model (reagent/atom false) + weeks-model (reagent/atom false) + interval-model (reagent/atom false) week-start-model (reagent/atom 2) - selected-days (reagent/atom {:M true :Tu true :W true :Th true :Fr true :Sa true :Su true}) ;model for all checkboxes - valid? (fn [day] (nth (mapv val @selected-days) (dec (cljs-time/day-of-week day))))] ;convert to vector, check if day should be disabled + selected-days (reagent/atom {:M true :Tu true :W true :Th true :Fr true :Sa true :Su true}) ;model for all checkboxes ;; TODO [GR-REMOVE] Haven't changed it but would prefer consistency of :Mo and :We + valid? (fn [day] (nth (mapv val @selected-days) (dec (cljs-time/day-of-week day))))] ;convert to vector, check if day should be disabled (fn [] - [v-box + [v-box :gap "10px" :children [[panel-title "[daterange ...]" - "src/re_com/datepicker.cljs" - "src/re_demo/datepicker.cljs"] + "src/re_com/daterange.cljs" ;; TODO [GR-REMOVE] corrected links + "src/re_demo/daterange.cljs"] [h-box :gap "100px" :children [[v-box @@ -72,6 +72,8 @@ :selectable-fn valid? :start-of-week @week-start-model :on-change #(reset! model-atom %)] + + ;; TODO [GR-REMOVE] because this line is just text, it doesn't need Flexbox power. You'll see we often make use is simple [:span]s for text only [h-box :align :center :children [[:code ":model"] @@ -79,6 +81,17 @@ (if @model-atom (str (unparse (formatter "dd MMM, yyyy") (:start @model-atom)) " ... " (unparse (formatter "dd MMM, yyyy") (:end @model-atom))) "nil"))]]] + + ;; TODO [GR-REMOVE] Here's the :span version (remove above when cleaning up) + [:span + [:code ":model"] + (str " is " + (if @model-atom ;; TODO [GR-REMOVE] Easier to read when formatted on multiple lines + (str + (unparse (formatter "dd MMM, yyyy") (:start @model-atom)) " ... " + (unparse (formatter "dd MMM, yyyy") (:end @model-atom))) + "nil"))] + [v-box :src (at) :gap "10px" @@ -90,15 +103,18 @@ :src (at) :style {:margin-top "0"} :level :level3 :label "Interactive Parameters"] - [checkbox :src (at) + [checkbox + :src (at) :model disabled-model :on-change #(swap! disabled-model not) :label [box :child [:code ":disabled?"]]] - [checkbox :src (at) + [checkbox + :src (at) :model today-model :on-change #(swap! today-model not) :label [box :child [:code ":show-today?"]]] - [checkbox :src (at) + [checkbox + :src (at) :model weeks-model :on-change #(swap! weeks-model not) :label [box :child [:code ":show-weeks?"]]] @@ -133,24 +149,22 @@ :children [[title :src (at) :level :level3 :label "Dropdown"] - [box :src (at) - :child "Attached to the same model and interactive paramters."] - [daterange-dropdown - :show-today? @today-model - :disabled? @disabled-model - :show-weeks? @weeks-model + [box + :src (at) + :child "Attached to the same model and interactive parameters."] + [daterange-dropdown ;; TODO [GR-REMOVE] I often like to align component args (but I haven't done it everywhere and there's no easy keyboard shortcut) + :show-today? @today-model + :disabled? @disabled-model + :show-weeks? @weeks-model :check-interval? @interval-model - :model model-atom - :selectable-fn valid? - :start-of-week @week-start-model - :on-change #(reset! model-atom %) - :placeholder "Select a range of dates"]]]]]]] + :model model-atom + :selectable-fn valid? + :start-of-week @week-start-model + :on-change #(reset! model-atom %) + :placeholder "Select a range of dates"]]]]]]] [parts-table "daterange" daterange-parts-desc]]]))) - - - (defn panel [] - [holder]) \ No newline at end of file + [holder])