Add contact code/topic validation #4752

vitvly · 2018-06-13T16:10:56Z

This PR adds contact code/topic validation, and design changes to add contact screen and chat list. Also add contact and tab selection buttons have been updated to look according to the design.

There are some design issues:

some elements of design depend on ~~TextInput issues react-native-desktop-qt#255~~ (resolved with TextInput got backgroundColor and fontSize properties react-native-desktop-qt#261) and Text component: ellipsizeMode issue react-native-desktop-qt#264
we don't have a popup menu component yet (related to PopupMenu component integration react-native-desktop-qt#256)
~~according to https://zpl.io/aBElz8L, we also need to implement on-hover event handler (Implement on-hover events and cursor shapes react-native-desktop-qt#257)~~ (decided to postpone it for now)

Design links: zpl.io/2ZgvMKw, https://zpl.io/aRqj4dv

cammellos

We tend to keep styling in styles.cljs files, instead of inlining, but otherwise shared codebase looks ok

cammellos · 2018-06-18T11:16:34Z

src/status_im/ui/screens/add_new/new_public_chat/subs.cljs

+  (fn [topic]
+    (when-not (spec/valid? ::db/topic topic)
+      "Group topic error")))
+


extra line at the end-of-file

rcullito

nice PR vitaliy! changes look good. in response to your comment about the popups not working, were you trying with the one in status-im.utils.utils or somewhere else? That one was able to work locally for me but I'm not sure if that would address your needs in this PR.

rcullito · 2018-06-18T11:07:50Z

src/status_im/ui/screens/add_new/new_public_chat/subs.cljs

+            [cljs.spec.alpha :as spec]))
+
+(re-frame/reg-sub
+  :new-topic-error-message


this is an interesting use of a subscription, to check a schema against the actual data 🤔 cool stuff!

@rcullito thanks for the pointer to show-popup in utils.utils! Unfortunately this is an alert-based popup, and we'd want to have multi-item clickable popups, more like proper context menus.

rcullito · 2018-06-18T11:12:03Z

src/status_im/ui/screens/desktop/main/add_new/views.cljs

+        (for [c contacts]
+        ^{:key (:whisper-identity c)}
+        [react/touchable-highlight {:on-press #(do
+                                                 (re-frame/dispatch [:set :contacts/new-identity (:whisper-identity c)])


might this PR also fix #4276? these remind me of the updated handlers that Andrea was mentioning.

yes that's one of the handlers you can use

nice, thanks

vitvly · 2018-06-18T14:52:05Z

Added separate styles ns.

flexsurfer

please use our clojure style guide , for example maps values should be aligned , etc, all styles maps should be moved to styles ns, use i18n for labels

flexsurfer · 2018-06-19T07:34:57Z

src/status_im/ui/screens/add_new/new_public_chat/subs.cljs

+  :<- [:get :public-group-topic]
+  (fn [topic]
+    (when-not (spec/valid? ::db/topic topic)
+      "Group topic error")))


flexsurfer · 2018-06-19T07:35:16Z

src/status_im/ui/screens/desktop/main/add_new/styles.cljs

@@ -0,0 +1,58 @@
+(ns status-im.ui.screens.desktop.main.add-new.styles)
+


wrong file formatting

flexsurfer · 2018-06-19T07:35:31Z

src/status_im/ui/screens/desktop/main/add_new/views.cljs

-     [react/view {:style {:height     90 :margin-horizontal 16 :margin-bottom 16 :background-color :white :border-radius 12}}
-                          ;:box-shadow "0 0.5px 4.5px 0 rgba(0, 0, 0, 0.04)"}}
-      [react/view {:style {:flex-direction :row :margin-horizontal 16 :margin-top 16}}
+                  contacts      [:all-added-people-contacts]


wrong formatting

flexsurfer · 2018-06-19T07:35:38Z

src/status_im/ui/screens/desktop/main/add_new/views.cljs

+      ^{:key "newcontact"}
+      [react/view {:style styles/new-contact-title}
+       [react/text {:style styles/new-contact-title-text}
+        "New Chat"]]


vkjr · 2018-06-19T09:48:13Z

src/status_im/ui/screens/desktop/main/add_new/views.cljs

@@ -70,4 +75,16 @@
       [react/touchable-highlight {:on-press #(when-not topic-error (re-frame/dispatch [:create-new-public-chat topic]))}
        [react/view {:style (merge styles/add-contact-button {:background-color (if topic-error "#eef2f5" "#4360df") })}

-         [react/text {:style (merge styles/add-contact-button-text {:color (if topic-error "#939ba1" :white)})} "Join public chat"]]]]]]))
+         [react/text {:style (merge styles/add-contact-button-text {:color (if topic-error "#939ba1" :white)})} "Join public chat"]]]]


topic-error here and above can be passed as an argument to style to make all color decision in styles.

Ah, great point, fixed!

churik · 2018-06-19T13:15:10Z

1) No "#" in "Join public chat"
expected (green) - actual (red):

If you start typing on the field "Join public chat" focus should appear right after "#" char.
Now "#" is displayed only as a placeholder
2) Public chats in chat lists look different in mobile and desktop versions
mobile (green) - desktop (red):

Expected - like in mobile(color icons with #, but this can be subject to discuss).
3) "Join public chat" or "Start chat" buttons are active after you already joined the chat
Steps:
click on +
select "status" from list (or insert valid contact code)
click on + again
Expected result: "Join public chat"( or "Start chat") button is not active
Actual result: "Join public chat" (or "Start chat")button is active
Video: http://take.ms/yc1g6
4) Long messages/names should be displayed with ... (issue in react-native)
5) If time of the last message contains more 7 chars (i.e. '10:31 AM') it is overlapping with chat area
6) Chat list is unresponsive after several scrolls up/down (randomly)
Prerequisites:
1-1 chats, public chats are started
Steps (not exact):
switch between chats
scroll up/down several times
Expected result: app is always responsive
Actual result: app is hanging
Video: http://take.ms/ItYiT
7) "No messages yet" is missing
when chat is empty,
expected (green) - actual (red):

EugeOrtiz · 2018-06-19T18:56:57Z

@siphiuel I can still see the old styles in the latest jenkins bundle https://jenkins.status.im/job/status-react/job/desktop/job/pull-requests/job/PR-4822/11/#
Should I download it from somewhere else?

churik · 2018-06-20T07:14:01Z

@EugeOrtiz yes, you should download it from particular PR build, in this case https://jenkins.status.im/job/status-react/job/desktop/job/pull-requests/view/change-requests/job/PR-4752/

vitvly · 2018-06-20T09:02:31Z

Used labels from i18n, aligned maps properly.

vkjr · 2018-06-20T09:05:03Z

src/status_im/ui/components/desktop/tabs.cljs

@@ -26,7 +27,7 @@
    [react/view {:style tabs.styles/tab-container}
     (let [icon (if active? icon-active icon-inactive)]
       [react/view
-        [icons/icon icon {:color (:color (tabs.styles/tab-icon active?))}]])
+        [icons/icon icon {:style {:tint-color (if active? "#4360df" "#6e777e")}}]])


@siphiuel, probably here we also should use colors namespace.

@vkjr I thought you wouldn't notice :) Fixed.

flexsurfer · 2018-06-20T10:46:35Z

src/status_im/ui/screens/add_new/new_chat/db.cljs

@@ -5,11 +5,12 @@
            [clojure.string :as string]))

 (defn- validate-pub-key [whisper-identity {:keys [address public-key]}]


i'm not sure about this change, how it affects mobile version

EugeOrtiz · 2018-06-20T17:57:50Z

vitvly · 2018-07-04T10:21:01Z

@Maxris much fewer you must have meant, not much more :)

maxhora · 2018-07-04T10:22:23Z

OMG, yes! :) Sorry

vitvly · 2018-07-05T10:28:35Z

Issues 1, 5, 8, 9 resolved.
Issue 10 - must be related to RN-desktop internals.
Issue 11 - list of default public chats has been updated in develop branch, after the recent merge of develop into desktop.

churik · 2018-07-05T17:41:27Z

5 still can be reproduced
(only with AM)
12, 13 are created separately (Crashes and CPU overload while fetching long history (>500 messages) #5012, Message history dates don't match real values #5070)
14 Last messages time is not aligned

So, only 2 small things and I'll move it to ready!

Some styling changes Add topic validation Wrap add-contact screen into scroll-view Improve chat list according to design Add color to tab icons Add color to add-contact icon Missing paren added Add background color to inputs in add-new screen Fix timestamp styling Remove extra empty line Move styles to separate ns Add styles for New Contact screen Add selected topics; improve chat item display Use colors from colors ns; style fns Fix formatting, add styles Use colors ns for tab selection buttons Use setNativeProps to force hash as first symbol in topic name Add :no-messages-yet label Alignment fixes in chat list Add timestamp style, disable 'add chat' buttons if no valid data Use color from chat-item map Formatting Use generic default-public-chats value Use proper last-message timestamp Fix '#' placeholder issues in public chat name field Fix timestamp sizing in chat list Add unviewed messages indicator Use proper chat name Fix double hash in public chat name Fix separator margin

vitvly · 2018-07-09T14:35:15Z

Issues 5 and 14 are related to #5104

vitvly · 2018-07-09T14:35:49Z

Item 12 and corresponding issue #5070 fixed

churik · 2018-07-10T10:31:26Z

Known:

Design issues moved to Design issues for New Chat and Join public chats #5118, No validation message if you are trying to add invalid contact code or Join public chat with invalid name #5117
Crash: added details to Crashes and CPU overload while fetching long history (>500 messages) #5012
RN-related: No validation message if you are trying to add invalid contact code or Join public chat with invalid name #5117, Public chat name is cut when it is selected #5113, Layout issues in chat list during frequent updates #5104
New Chat and Join public chat in separate popup: Apply design for new menu and screens for desktop "+" button #4434

vitvly added the desktop label Jun 13, 2018

vitvly self-assigned this Jun 13, 2018

vitvly changed the title ~~[WIP] Add contact code/topic validation~~ Add contact code/topic validation Jun 18, 2018

vitvly requested review from rcullito, cammellos and vkjr June 18, 2018 10:54

cammellos approved these changes Jun 18, 2018

View reviewed changes

cammellos reviewed Jun 18, 2018

View reviewed changes

rcullito approved these changes Jun 18, 2018

View reviewed changes

cammellos requested review from rcullito and flexsurfer June 18, 2018 11:16

flexsurfer requested changes Jun 19, 2018

View reviewed changes

vkjr reviewed Jun 19, 2018

View reviewed changes

vkjr approved these changes Jun 19, 2018

View reviewed changes

churik mentioned this pull request Jun 19, 2018

Desktop app starts in mobile resolution #4824

Closed

churik added the Tested - Issues label Jun 19, 2018

churik mentioned this pull request Jun 20, 2018

UI/UX fixes for issue #4791 (Chat design) #4839

Closed

10 tasks

vkjr reviewed Jun 20, 2018

View reviewed changes

flexsurfer approved these changes Jun 20, 2018

View reviewed changes

rcullito approved these changes Jun 20, 2018

View reviewed changes

vitvly force-pushed the fix/4250-add-contact-code-validation branch from 0169e29 to c15cf73 Compare July 5, 2018 10:01

Vitaliy Vlasov added 2 commits July 9, 2018 15:18

Fix React keys for datemark message types (and some others)

9af908b

vitvly force-pushed the fix/4250-add-contact-code-validation branch from 571985e to 9af908b Compare July 9, 2018 13:48

vitvly mentioned this pull request Jul 9, 2018

Layout issues in chat list during frequent updates #5104

Closed

churik added Tested - OK and removed Tested - Issues labels Jul 10, 2018

vitvly merged commit 179cc96 into desktop Jul 10, 2018

This was referenced Jul 12, 2018

Message history is overlapping when you have several chats with history>1 day (when switching between chats) #4995

Closed

Selected tab is not highlighted #4266

Closed

rasom deleted the fix/4250-add-contact-code-validation branch September 3, 2018 09:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add contact code/topic validation #4752

Add contact code/topic validation #4752

vitvly commented Jun 13, 2018 •

edited

Loading

cammellos left a comment

cammellos Jun 18, 2018

rcullito left a comment

rcullito Jun 18, 2018

vitvly Jun 18, 2018

rcullito Jun 18, 2018

cammellos Jun 18, 2018

rcullito Jun 18, 2018

vitvly commented Jun 18, 2018

flexsurfer left a comment

flexsurfer Jun 19, 2018

flexsurfer Jun 19, 2018

flexsurfer Jun 19, 2018

flexsurfer Jun 19, 2018

vkjr Jun 19, 2018

vitvly Jun 20, 2018

churik commented Jun 19, 2018 •

edited

Loading

EugeOrtiz commented Jun 19, 2018 •

edited by ghost

Loading

churik commented Jun 20, 2018 •

edited by ghost

Loading

vitvly commented Jun 20, 2018

vkjr Jun 20, 2018

vitvly Jun 20, 2018

flexsurfer Jun 20, 2018

EugeOrtiz commented Jun 20, 2018 •

edited

Loading

vitvly commented Jul 4, 2018

maxhora commented Jul 4, 2018

vitvly commented Jul 5, 2018

churik commented Jul 5, 2018 •

edited

Loading

vitvly commented Jul 9, 2018

vitvly commented Jul 9, 2018

churik commented Jul 10, 2018 •

edited

Loading

		@@ -0,0 +1,58 @@
		(ns status-im.ui.screens.desktop.main.add-new.styles)

		@@ -5,11 +5,12 @@
		[clojure.string :as string]))

		(defn- validate-pub-key [whisper-identity {:keys [address public-key]}]

Add contact code/topic validation #4752

Add contact code/topic validation #4752

Conversation

vitvly commented Jun 13, 2018 • edited Loading

cammellos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcullito left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitvly commented Jun 18, 2018

flexsurfer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

churik commented Jun 19, 2018 • edited Loading

EugeOrtiz commented Jun 19, 2018 • edited by ghost Loading

churik commented Jun 20, 2018 • edited by ghost Loading

vitvly commented Jun 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EugeOrtiz commented Jun 20, 2018 • edited Loading

vitvly commented Jul 4, 2018

maxhora commented Jul 4, 2018

vitvly commented Jul 5, 2018

churik commented Jul 5, 2018 • edited Loading

vitvly commented Jul 9, 2018

vitvly commented Jul 9, 2018

churik commented Jul 10, 2018 • edited Loading

vitvly commented Jun 13, 2018 •

edited

Loading

churik commented Jun 19, 2018 •

edited

Loading

EugeOrtiz commented Jun 19, 2018 •

edited by ghost

Loading

churik commented Jun 20, 2018 •

edited by ghost

Loading

EugeOrtiz commented Jun 20, 2018 •

edited

Loading

churik commented Jul 5, 2018 •

edited

Loading

churik commented Jul 10, 2018 •

edited

Loading