diff --git a/src/status_im/chat/styles/message/datemark.cljs b/src/status_im/chat/styles/message/datemark.cljs index 1953439396..a4aea47a5f 100644 --- a/src/status_im/chat/styles/message/datemark.cljs +++ b/src/status_im/chat/styles/message/datemark.cljs @@ -7,7 +7,7 @@ (def datemark {:opacity 0.5 - :margin-top 20 + :margin-top 16 :height 20}) (def datemark-text diff --git a/src/status_im/chat/styles/message/message.cljs b/src/status_im/chat/styles/message/message.cljs index 981dcc13fe..8ba24bf232 100644 --- a/src/status_im/chat/styles/message/message.cljs +++ b/src/status_im/chat/styles/message/message.cljs @@ -20,12 +20,10 @@ :height 16}) (defn message-padding-top - [{:keys [first-in-date? same-author? same-direction?]}] - (cond - first-in-date? 20 - same-author? 8 - same-direction? 16 - :else 24)) + [{:keys [first-in-group?]}] + (if first-in-group? + 8 + 4)) (def message-empty-spacing {:height 16}) @@ -50,6 +48,14 @@ :align-self align :align-items align}))) +(def message-timestamp + {:margin-left 5 + :margin-right 5 + :margin-bottom -2 + :color colors/gray + :opacity 0.5 + :align-self :flex-end}) + (def selected-message {:margin-top 18 :margin-left 40 @@ -60,6 +66,9 @@ (merge {:flex-direction :column} (last-message-padding message))) +(defn timestamp-content-wrapper [{:keys [outgoing]}] + {:flex-direction (if outgoing :row-reverse :row)}) + (defn group-message-view [outgoing] (let [align (if outgoing :flex-end :flex-start)] @@ -71,11 +80,11 @@ (def delivery-status {:align-self :flex-end - :padding-right 56}) + :padding-right 22}) (def message-author {:width photo-size - :align-self :flex-start}) + :align-self :flex-end}) (def photo {:border-radius (/ photo-size 2) diff --git a/src/status_im/chat/subs.cljs b/src/status_im/chat/subs.cljs index 5d9cf95625..b96abaa8bf 100644 --- a/src/status_im/chat/subs.cljs +++ b/src/status_im/chat/subs.cljs @@ -84,7 +84,7 @@ (fn [{:keys [messages]} [_ message-id]] (get messages message-id))) -(defn- partition-by-datemark +(defn- intersperse-datemark "Reduce step which expects the input list of messages to be sorted by clock value. It makes best effort to group them by day. We cannot sort them by :timestamp, as that represents the clock of the sender @@ -100,80 +100,120 @@ so we bucket both in 1999-12-31" [{:keys [acc last-timestamp last-datemark]} {:keys [timestamp datemark] :as msg}] - (if (or (empty? acc) ; initial element - (and (not= last-datemark datemark) ; not the same day - (< timestamp last-timestamp))) ; not out-of-order - {:last-timestamp timestamp - :last-datemark datemark - :acc (conj acc [datemark [msg]])} ; add new datemark group - {:last-timestamp (max timestamp last-timestamp) - :last-datemark last-datemark - :acc (conj (pop acc) (update (peek acc) 1 conj msg))})) ; conj to the last element + (cond (empty? acc) ; initial element + {:last-timestamp timestamp + :last-datemark datemark + :acc (conj acc msg)} -(defn message-datemark-groups - "Transforms map of messages into sequence of `[datemark messages]` tuples, where - messages with particular datemark are sorted according to their clock-values." + (and (not= last-datemark datemark) ; not the same day + (< timestamp last-timestamp)) ; not out-of-order + {:last-timestamp timestamp + :last-datemark datemark + :acc (conj acc {:value last-datemark ; intersperse datemark message + :type :datemark} + msg)} + :else + {:last-timestamp (max timestamp last-timestamp) ; use last datemark + :last-datemark last-datemark + :acc (conj acc (assoc msg :datemark last-datemark))})) + +(defn sort-messages + "Remove hidden messages and sort by clock-value desc, breaking ties by message id" [id->messages] - (let [sorted-messages (->> id->messages - vals - (sort-by (juxt (comp unchecked-negate :clock-value) :message-id))) ; sort-by clock in reverse order, break ties by :message-id field - remove-hidden-xf (filter :show?) - add-datemark-xf (map (fn [{:keys [timestamp] :as msg}] - (assoc msg :datemark (time/day-relative timestamp))))] - (-> (transduce (comp remove-hidden-xf - add-datemark-xf) - (completing partition-by-datemark) - {:acc []} - sorted-messages) - :acc))) + (->> id->messages + vals + (filter :show?) + (sort-by (juxt (comp unchecked-negate :clock-value) :message-id)))) + +(defn- add-datemark [{:keys [timestamp] :as msg}] + (assoc msg :datemark (time/day-relative timestamp))) + +(defn- add-timestamp [{:keys [timestamp] :as msg}] + (assoc msg :timestamp-str (time/timestamp->time timestamp))) + +(defn intersperse-datemarks + "Add a datemark in between an ordered seq of messages when two datemarks are not + the same. Ignore messages with out-of-order timestamps" + [messages] + (when (seq messages) + (let [messages-with-datemarks (transduce (comp + (map add-datemark) + (map add-timestamp)) + (completing intersperse-datemark :acc) + {:acc []} + messages)] + ; Append last datemark + (conj messages-with-datemarks {:value (:datemark (peek messages-with-datemarks)) + :type :datemark})))) + +(defn- set-previous-message-first-in-group [stream] + (conj (pop stream) (assoc (peek stream) :first-in-group? true))) + +; any message that comes after this amount of ms will be grouped separately +(def ^:private group-ms 60000) + +(defn add-positional-metadata + "Reduce step which adds positional metadata to a message and conditionally + update the previous message with :first-in-group?." + [{:keys [stream last-outgoing-seen]} + {:keys [type from datemark outgoing timestamp] :as message}] + (let [previous-message (peek stream) + ; Was the previous message from a different author or this message + ; comes after x ms + last-in-group? (or (not= from (:from previous-message)) + (> (- (:timestamp previous-message) timestamp) group-ms)) + same-direction? (= outgoing (:outgoing previous-message)) + ; Have we seen an outgoing message already? + last-outgoing? (and (not last-outgoing-seen) + outgoing) + datemark? (= :datemark (:type message)) + ; If this is a datemark or this is the last-message of a group, + ; then the previous message was the first + previous-first-in-group? (or datemark? + last-in-group?) + new-message (assoc message + :same-direction? same-direction? + :last-in-group? last-in-group? + :last-outgoing? last-outgoing?)] + {:stream (cond-> stream + previous-first-in-group? + ; update previuous message if necessary + set-previous-message-first-in-group + + :always + (conj new-message)) + ; mark the last message sent by the user + :last-outgoing-seen (or last-outgoing-seen last-outgoing?)})) + +(defn messages-stream + "Enhances the messages in message sequence interspersed with datemarks + with derived stream context information, like: + `:first-in-group?`, `last-in-group?`, `:same-direction?`, `:last?` and `:last-outgoing?` flags." + [ordered-messages] + (when (seq ordered-messages) + (let [initial-message (first ordered-messages) + message-with-metadata (assoc initial-message + :last-in-group? true + :last? true + :last-outgoing? (:outgoing initial-message))] + (->> (rest ordered-messages) + (reduce add-positional-metadata + {:stream [message-with-metadata] + :last-outgoing-seen (:last-outgoing? message-with-metadata)}) + :stream)))) (reg-sub - :get-chat-message-datemark-groups + :get-ordered-chat-messages (fn [[_ chat-id]] (subscribe [:get-chat chat-id])) (fn [{:keys [messages]}] - (message-datemark-groups messages))) - -(defn messages-stream - "Transforms message-datemark-groups into flat sequence of messages interspersed with - datemark messages. - Additionaly enhances the messages in message sequence with derived stream context information, - like `:same-author?`, `:same-direction?`, `:last?` and `:last-outgoing?` flags. " - [message-datemark-groups] - (if (seq message-datemark-groups) - (let [messages-seq (mapcat second message-datemark-groups) - {last-message-id :message-id} (first messages-seq) - {last-outgoing-message-id :message-id} (->> messages-seq - (filter :outgoing) - first)] - (->> message-datemark-groups - (mapcat (fn [[datemark messages]] - (let [prepared-messages (into [] - (map (fn [previous-message - {:keys [message-id] :as message} - next-message] - (assoc message - :same-author? (= (:from message) - (:from previous-message)) - :same-direction? (= (:outgoing message) - (:outgoing previous-message)) - :last-by-same-author? (not= (:from message) - (:from next-message)) - :last? (= message-id - last-message-id) - :last-outgoing? (= message-id - last-outgoing-message-id))) - (concat (rest messages) '(nil)) - messages - (concat '(nil) (butlast messages))))] - (conj prepared-messages {:type :datemark - :value datemark})))))))) + (sort-messages messages))) (reg-sub :get-current-chat-messages :<- [:get-current-chat] (fn [{:keys [messages]}] - (-> messages message-datemark-groups messages-stream))) + (-> messages sort-messages intersperse-datemarks messages-stream))) (reg-sub :get-commands-for-chat @@ -335,8 +375,8 @@ (reg-sub :get-last-message (fn [[_ chat-id]] - (subscribe [:get-chat-message-datemark-groups chat-id])) - (comp first second first)) + (subscribe [:get-ordered-chat-messages chat-id])) + first) (reg-sub :chat-animations diff --git a/src/status_im/chat/views/message/message.cljs b/src/status_im/chat/views/message/message.cljs index 132a3f1d1d..aa456be85d 100644 --- a/src/status_im/chat/views/message/message.cljs +++ b/src/status_im/chat/views/message/message.cljs @@ -274,6 +274,9 @@ (letsubs [{:keys [photo-path]} [:get-current-account]] (photo from photo-path))) +(defview message-timestamp [t] + [react/text {:style style/message-timestamp} t]) + (defview message-author-name [from message-username] (letsubs [username [:get-contact-name-by-identity from]] [react/text {:style style/message-author-name} (or username @@ -281,20 +284,21 @@ (gfycat/generate-gfy from))])) ; TODO: We defensively generate the name for now, to be revisited when new protocol is defined (defn message-body - [{:keys [last-outgoing? last-by-same-author? message-type same-author? from outgoing group-chat username] :as message} content] + [{:keys [timestamp-str last-outgoing? last-in-group? message-type first-in-group? from outgoing group-chat username] :as message} content] [react/view (style/group-message-wrapper message) [react/view (style/message-body message) - [react/view style/message-author - (when last-by-same-author? - (if outgoing - [my-photo from] + (when (not outgoing) + [react/view style/message-author + (when last-in-group? [react/touchable-highlight {:on-press #(re-frame/dispatch [:show-profile from])} [react/view - [member-photo from]]]))] + [member-photo from]]])]) [react/view (style/group-message-view outgoing) - (when-not same-author? + (when first-in-group? [message-author-name from username]) - content]] + [react/view {:style (style/timestamp-content-wrapper message)} + content + [message-timestamp timestamp-str]]]] (when last-outgoing? [react/view style/delivery-status (if (= message-type :group-user-message) diff --git a/src/status_im/utils/datetime.cljs b/src/status_im/utils/datetime.cljs index 5819a8a6b5..468fea5de5 100644 --- a/src/status_im/utils/datetime.cljs +++ b/src/status_im/utils/datetime.cljs @@ -32,21 +32,23 @@ loc (get goog.i18n (str "DateTimeSymbols_" name-first))] (or loc goog.i18n.DateTimeSymbols_en)))) -;; Closure does not have an enum for datetime formats -(def short-date-time-format 10) -(def short-date-format 2) +(def medium-date-time-format (.-MEDIUM_DATETIME goog.i18n.DateTimeFormat.Format)) +(def medium-date-format (.-MEDIUM_DATE goog.i18n.DateTimeFormat.Format)) +(def short-time-format (.-SHORT_TIME goog.i18n.DateTimeFormat.Format)) (defn mk-fmt [locale format] (goog.i18n.DateTimeFormat. format (locale-symbols locale))) (def date-time-fmt - (mk-fmt status-im.i18n/locale short-date-time-format)) + (mk-fmt status-im.i18n/locale medium-date-time-format)) (def date-fmt - (mk-fmt status-im.i18n/locale short-date-format)) + (mk-fmt status-im.i18n/locale medium-date-format)) +(def time-fmt + (mk-fmt status-im.i18n/locale short-time-format)) (defn- to-str [ms old-fmt-fn yesterday-fmt-fn today-fmt-fn] (let [date (from-long ms) - local (plus date time-zone-offset) + local (plus date time-zone-offset) ; this is wrong, it uses the current timezone offset, regardless of DST today (t/today-at-midnight) yesterday (plus today (days -1))] (cond @@ -58,7 +60,7 @@ (to-str ms #(.format date-fmt %) #(label :t/datetime-yesterday) - #(unparse (formatters :hour-minute) %))) + #(.format time-fmt %))) (defn day-relative [ms] (to-str ms @@ -72,9 +74,9 @@ (plus time-zone-offset)))) (defn timestamp->time [ms] - (unparse (formatter "HH:mm") (-> ms - from-long - (plus time-zone-offset)))) + (.format time-fmt (-> ms + from-long + (plus time-zone-offset)))) (defn timestamp->date-key [ms] (keyword (unparse (formatter "YYYYMMDD") (-> ms diff --git a/test/cljs/status_im/test/chat/subs.cljs b/test/cljs/status_im/test/chat/subs.cljs index f22ee863eb..18b9422573 100644 --- a/test/cljs/status_im/test/chat/subs.cljs +++ b/test/cljs/status_im/test/chat/subs.cljs @@ -2,56 +2,135 @@ (:require [cljs.test :refer-macros [deftest is testing]] [status-im.chat.subs :as s])) - -(defn messages-ordered? [messages] - (let [clock-values (map :clock-value messages)] - (= (-> clock-values sort reverse) clock-values))) - (deftest test-message-datemark-groups - (testing "it orders a map of messages by clock-values when all on the same day (by sender timestamp)" - (let [datemark "Jan 1, 1970" - message-1 {:show? true - :timestamp 0 + (testing "it orders a map of messages by clock-values desc, breaking ties by message-id asc and removing hidden messages" + (let [message-1 {:show? true + :message-id "doesn't matter 1" :clock-value 1} message-2 {:show? true - :timestamp 0 + :message-id "doesn't matter 2" :clock-value 2} message-3 {:show? true - :timestamp 0 - :clock-value 3} - unordered-messages {2 message-2 - 1 message-1 - 3 message-3} - [[actual-datemark actual-messages]] (s/message-datemark-groups unordered-messages)] - - (is (= datemark actual-datemark)) - (is (= 3 (count actual-messages))) - (is (messages-ordered? actual-messages)))) - - (testing "it mantains the order even when timestamps are across days" - (let [datemark-day-1 "Jan 1, 2000" - datemark-day-2 "Dec 31, 1999" - message-1 {:show? true - :timestamp 946641600000 ; 1999 - :clock-value 1} - message-2 {:show? true - :timestamp 946728000000 ; 2000 this will displayed in 1999 - :clock-value 2} - message-3 {:show? true - :timestamp 946641600000 ; 1999 + :message-id "does matter 2" :clock-value 3} message-4 {:show? true - :timestamp 946728000000 ; 2000 - :clock-value 4} - unordered-messages {2 message-2 - 1 message-1 - 4 message-4 - 3 message-3} - [[actual-dm-1 actual-msg-1] - [actual-dm-2 actual-msg-2]] (s/message-datemark-groups unordered-messages)] + :message-id "does matter 1" + :clock-value 3} + hidden-message {:show? false + :clock-value 1} + unordered-messages (->> [message-1 + message-2 + message-3 + message-4 + hidden-message] + (map (juxt :message-id identity)) + shuffle ; clojure maps are sorted for n <= 32 + (into {}))] + (is (= [message-4 + message-3 + message-2 + message-1] (s/sort-messages unordered-messages)))))) - (is (= datemark-day-1 actual-dm-1)) - (is (= datemark-day-2 actual-dm-2)) - (is (= 1 (count actual-msg-1))) - (is (= 3 (count actual-msg-2))) - (is (messages-ordered? (concat actual-msg-1 actual-msg-2)))))) +(deftest intersperse-datemarks + (testing "it mantains the order even when timestamps are across days" + (let [message-1 {:timestamp 946641600000} ; 1999} + message-2 {:timestamp 946728000000} ; 2000 this will displayed in 1999 + message-3 {:timestamp 946641600000} ; 1999 + message-4 {:timestamp 946728000000} ; 2000 + ordered-messages [message-4 + message-3 + message-2 + message-1] + [m1 d1 m2 m3 m4 d2] (s/intersperse-datemarks ordered-messages)] + (is (= "Jan 1, 2000" + (:datemark m1))) + (is (= {:type :datemark + :value "Jan 1, 2000"} d1)) + (is (= "Dec 31, 1999" + (:datemark m2) + (:datemark m3) + (:datemark m4))) + (is (= {:type :datemark + :value "Dec 31, 1999"} d2))))) + +(deftest message-stream-tests + (testing "messages with no interspersed datemarks" + (let [m1 {:from "1" + :datemark "a" + :outgoing false} + m2 {:from "2" + :datemark "a" + :outgoing true} + m3 {:from "2" + :datemark "a" + :outgoing true} + dm1 {:type :datemark + :value "a"} + messages [m1 m2 m3 dm1] + [actual-m1 + actual-m2 + actual-m3] (s/messages-stream messages)] + (testing "it marks only the first message as :last?" + (is (:last? actual-m1)) + (is (not (:last? actual-m2))) + (is (not (:last? actual-m3)))) + (testing "it marks the first outgoing message as :last-outgoing?" + (is (not (:last-outgoing? actual-m1))) + (is (:last-outgoing? actual-m2)) + (is (not (:last-outgoing? actual-m3)))) + (testing "it marks the message with :same-direction? when from the same author" + (is (not (:same-direction? actual-m1))) + (is (not (:same-direction? actual-m2)) + (is (:same-direction? actual-m3)))) + (testing "it marks messages from the same author next to another with :first-in-group?" + (is (:first-in-group? actual-m1)) + (is (not (:first-in-group? actual-m2))) + (is (:first-in-group? actual-m3))) + (testing "it marks the last message from the same author with :last-in-group?" + (is (:last-in-group? actual-m1)) + (is (:last-in-group? actual-m2)) + (is (not (:last-in-group? actual-m3)))))) + (testing "messages with interspersed datemarks" + (let [m1 {:from "2" ; first & last in group + :timestamp 63000 + :outgoing true} + dm1 {:type :datemark + :value "a"} + m2 {:from "2" ; first & last in group as more than 1 minute after previous message + :timestamp 62000 + :outgoing false} + m3 {:from "2" ; last in group + :timestamp 1 + :outgoing false} + m4 {:from "2" ; first in group + :timestamp 0 + :outgoing false} + dm2 {:type :datemark + :value "b"} + messages [m1 dm1 m2 m3 m4 dm2] + [actual-m1 + _ + actual-m2 + actual-m3 + actual-m4 + _] (s/messages-stream messages)] + (testing "it marks the first outgoing message as :last-outgoing?" + (is (:last-outgoing? actual-m1)) + (is (not (:last-outgoing? actual-m2))) + (is (not (:last-outgoing? actual-m3))) + (is (not (:last-outgoing? actual-m4)))) + (testing "it resets :same-direction? after a datemark" + (is (not (:same-direction? actual-m1))) + (is (not (:same-direction? actual-m2)) + (is (:same-direction? actual-m3))) + (is (:same-direction? actual-m4))) + (testing "it sets :first-in-group? after a datemark" + (is (:first-in-group? actual-m1)) + (is (:first-in-group? actual-m4))) + (testing "it sets :first-in-group? if more than 60s have passed since last message" + (is (:first-in-group? actual-m2))) + (testing "it sets :last-in-group? after a datemark" + (is (:last-in-group? actual-m1)) + (is (:last-in-group? actual-m2)) + (is (:last-in-group? actual-m3)) + (is (not (:last-in-group? actual-m4))))))) diff --git a/test/cljs/status_im/test/runner.cljs b/test/cljs/status_im/test/runner.cljs index 539fbada45..c4264a872b 100644 --- a/test/cljs/status_im/test/runner.cljs +++ b/test/cljs/status_im/test/runner.cljs @@ -9,6 +9,7 @@ [status-im.test.profile.events] [status-im.test.bots.events] [status-im.test.chat.models.input] + [status-im.test.chat.subs] [status-im.test.chat.views.message] [status-im.test.i18n] [status-im.test.protocol.web3.inbox] @@ -37,6 +38,7 @@ (doo-tests 'status-im.test.utils.async 'status-im.test.chat.events + 'status-im.test.chat.subs 'status-im.test.accounts.events 'status-im.test.contacts.events 'status-im.test.profile.events diff --git a/test/cljs/status_im/test/utils/datetime.cljs b/test/cljs/status_im/test/utils/datetime.cljs index 1514d69a48..d9d97544e9 100644 --- a/test/cljs/status_im/test/utils/datetime.cljs +++ b/test/cljs/status_im/test/utils/datetime.cljs @@ -4,7 +4,7 @@ [cljs-time.core :as t])) (defn match [name symbols] - (is (identical? (.-dateTimeSymbols_ (d/mk-fmt name d/short-date-format)) + (is (identical? (.-dateTimeSymbols_ (d/mk-fmt name d/medium-date-format)) symbols))) (deftest date-time-formatter-test @@ -23,29 +23,30 @@ (deftest to-short-str-today-test (with-redefs [t/*ms-fn* (constantly epoch-plus-3d) + d/time-fmt (d/mk-fmt "us" d/short-time-format) d/time-zone-offset (t/period :hours 0)] - (is (= (d/to-short-str epoch-plus-3d) "00:00")))) + (is (= (d/to-short-str epoch-plus-3d) "12:00 AM")))) (deftest to-short-str-before-yesterday-us-test (with-redefs [t/*ms-fn* (constantly epoch-plus-3d) d/time-zone-offset (t/period :hours 0) - d/date-fmt (d/mk-fmt "us" d/short-date-format)] + d/date-fmt (d/mk-fmt "us" d/medium-date-format)] (is (= (d/to-short-str epoch) "Jan 1, 1970")))) (deftest to-short-str-before-yesterday-nb-test (with-redefs [d/time-zone-offset (t/period :hours 0) - d/date-fmt (d/mk-fmt "nb-NO" d/short-date-format) + d/date-fmt (d/mk-fmt "nb-NO" d/medium-date-format) t/*ms-fn* (constantly epoch-plus-3d)] (is (= (d/to-short-str epoch) "1. jan. 1970")))) (deftest day-relative-before-yesterday-us-test (with-redefs [t/*ms-fn* (constantly epoch-plus-3d) d/time-zone-offset (t/period :hours 0) - d/date-fmt (d/mk-fmt "us" d/short-date-time-format)] + d/date-fmt (d/mk-fmt "us" d/medium-date-time-format)] (is (= (d/day-relative epoch) "Jan 1, 1970, 12:00:00 AM")))) (deftest day-relative-before-yesterday-nb-test (with-redefs [t/*ms-fn* (constantly epoch-plus-3d) d/time-zone-offset (t/period :hours 0) - d/date-fmt (d/mk-fmt "nb-NO" d/short-date-time-format)] + d/date-fmt (d/mk-fmt "nb-NO" d/medium-date-time-format)] (is (= (d/day-relative epoch) "1. jan. 1970, 00:00:00"))))