New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Share contact code #4908

Merged

vitvly merged 6 commits into desktop from fix/4438-share-contact-code

Jul 9, 2018

Contributor

vitvly commented Jun 22, 2018 •

edited

Loading

Fix #4438 , #4992

This fix implements "Share contact code" window. Temporarily not in a modal, but in a right sidebar pane (due to status-im/react-native-desktop-qt#258)

vitvly added the desktop label

vitvly changed the base branch from develop to desktop

June 22, 2018 11:08

flexsurfer assigned vitvly

vitvly force-pushed the fix/4438-share-contact-code branch from 09cefcf to d0c80bc Compare

June 25, 2018 13:54

vitvly requested review from rcullito, flexsurfer and maxhora

June 26, 2018 07:28

flexsurfer requested changes

View reviewed changes

src/status_im/ui/screens/desktop/main/tabs/profile/styles.cljs Outdated

+              (def qr-code-copy-text
+                {:font-size 16
+                 :color colors/white})

Member

flexsurfer Jun 26, 2018

indentation

src/status_im/ui/screens/desktop/main/tabs/profile/views.cljs Outdated

@@ @@ -25,6 +29,34 @@ @@
                                :ellipsizeMode   :middle}
                    value]]]])
+              (views/defview qr-code []
+                (views/letsubs [current-account [:get-current-account]]

Member

flexsurfer Jun 26, 2018

{:keys [public-key]} [:get-current-account]

src/status_im/ui/screens/desktop/main/tabs/profile/views.cljs Outdated

+                      [react/text {:style styles/qr-code-title}
+                       (string/replace (i18n/label :qr-code-public-key-hint) "\n" "")]
+                      [react/view {:style styles/qr-code}
+                       [qr-code-viewer/qr-code {:value public-key :size 130} #_{:style styles/qr-code}]]

Member

flexsurfer Jun 26, 2018

could you remove commented part or add TODO

src/status_im/ui/screens/desktop/main/tabs/profile/views.cljs Outdated

+                       [qr-code-viewer/qr-code {:value public-key :size 130} #_{:style styles/qr-code}]]
+                      [react/text {:style styles/qr-code-text}
+                       public-key]
+                      [react/touchable-highlight {:on-press #(react/copy-to-clipboard public-key)}

Member

flexsurfer Jun 26, 2018 •

edited

Loading

better to move react/copy-to-clipboard to effect, dispatch event and use custom fx in event

Member

flexsurfer Jun 26, 2018

views should be pure

src/status_im/ui/screens/desktop/main/tabs/profile/views.cljs Outdated

@@ @@ -38,7 +70,8 @@ @@
                     [profile-badge current-account]]
                    [react/view {:style {:height 1 :background-color "#e8ebec" :margin-horizontal 16}}]
                    [react/view
-                    [my-profile-info current-account]]
+                    [share-contact-code]
+                    #_[my-profile-info current-account]]

Member

flexsurfer Jun 26, 2018

please remove or add TODO

Contributor Author

vitvly commented Jun 26, 2018

Thank you for review @flexsurfer ! I've made the requested changes, and moved the clipboard copy functionality to the custom fx handler.

flexsurfer reviewed

View reviewed changes

src/status_im/ui/screens/desktop/main/tabs/profile/views.cljs Outdated

    
                       [react/view {:style styles/qr-code-copy}

                        [react/text {:style styles/qr-code-copy-text}

                         (i18n/label :copy-qr)]]]]])))

                (views/letsubs [{:keys [public-key] :as current-account} [:get-current-account]]

Member

flexsurfer Jun 26, 2018

but you don't use current-account why do you need it? just wondering

Contributor Author

vitvly Jun 26, 2018

sorry, you're right, fixed it, thought it was referred to in the body of fn.

flexsurfer approved these changes

View reviewed changes

rcullito approved these changes

View reviewed changes

Contributor

rcullito left a comment

nice workaround for the modal issue vitaliy!

does desktop have any current working implementation that you've seen for this type of component, or are we all the way down to the popup?

src/status_im/ui/screens/profile/events.cljs Outdated

    
              (re-frame/reg-fx

                :copy-to-clipboard

                (fn [value]

Contributor

rcullito Jun 26, 2018

is there any interceptor, etc, that causes the value to be the first parameter here instead of the event name?

Member

flexsurfer Jun 26, 2018

it's effect, not event https://github.com/Day8/re-frame/blob/master/docs/Effects.md#extensible-side-effects

Contributor

rcullito Jun 26, 2018

ah I see. completely new effect being introduced. thanks for the link.

EugeOrtiz commented Jun 27, 2018

I cannot find how to see this in the latest builds, should I download a special one? Or click on something? 🤔

Contributor

maxhora commented Jun 27, 2018

@EugeOrtiz you should check Jenkins build status for it (Click on Details for Jenkins check in the bottom of PR page), but since it's in failed state the build for this PR is not available yet.

maxhora approved these changes

View reviewed changes

vitvly force-pushed the fix/4438-share-contact-code branch from 185cfc6 to 66091be Compare

June 28, 2018 09:04

vitvly force-pushed the fix/4438-share-contact-code branch from ed5a55f to 5631f7e Compare

June 28, 2018 17:41

Member

churik commented Jun 29, 2018 •

edited

Loading

Screen (all issues are marked):

(1) No QR is displayed
(2) After copying contact code no message "Copied to clipboard" (according to zpl.io/VxY83em )
(3) Text is not centered
(4, 5) Grey lines are displayed on top and bottom button edges; "Share my contact code" button is not centered.
(6) "Close" button is not working (is it not Modal component issues react-native-desktop-qt#258 ?)

churik added the Tested - Issues label

churik mentioned this pull request

UI/UX fixes for User Profile (left panel) #4986

Closed

6 tasks

Contributor Author

vitvly commented Jun 29, 2018

1 - Weird, cannot reproduce.
3, 4, 5 - fixed
6 - yes, close button there is not functional because there is nothing to close (there is no modal)

EugeOrtiz commented Jun 29, 2018

@churik some of your indications are also here #4986

EugeOrtiz commented Jun 29, 2018 •

edited by churik

Loading

Adding:

"Share my contact code" button should be aligned in the center of the left panel:

Close icon ("X") shouldn't be displayed if it doesn't work.
Right panel should disappear (if it cannot be closed) when I navigate to "Home" tab.

Vitaliy Vlasov added 3 commits

July 5, 2018 15:44


          Add share-contact-code

8f72837

Qr code copy window

Copy code on-press handler

Add profile styles

Alignments, obsolete style removed

Add qr code close icon

Fix typo

Add copy-to-clipboard event handler and fx

Remove obsolete current-account binding

Formatting

Remove unneeded horizontal lines

Center qr code text

Center 'Share my contact code' button

Add green 'Copied to clipboard' tooltip view


          Add fading for qr code copied tooltip

63f7898


          Always clear intervals

4fd1c24

vitvly force-pushed the fix/4438-share-contact-code branch from f692f1e to 4fd1c24 Compare

July 5, 2018 17:05

Vitaliy Vlasov added 3 commits

July 6, 2018 13:38


          Change profile view styling

4f3e7a5


          Refresh right pane when navigating to a different tab

f912dd1


          Increase left pane width to 340

1be0062

Contributor Author

vitvly commented Jul 6, 2018

#2 - added a tooltip that fades away after a couple of seconds
Removed close button.
Also changed profile view in order to address #4992 .
Right panel disappears when user navigates to a different tab.

churik mentioned this pull request

Implement User Profile screen #4992

Closed

3 tasks

Member

churik commented Jul 9, 2018

Created #5098, #5099

churik added Tested - OK and removed Tested - Issues labels

vitvly merged commit 94df17e into desktop

rasom deleted the fix/4438-share-contact-code branch

September 3, 2018 09:47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels