Skip to content
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.

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

Fix QR share screen for community channel #19792

Merged
merged 13 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/quo/components/share/share_qr_code/schema.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
[:type [:= :profile]]
[:profile-picture [:maybe :schema.quo/profile-picture-source]]])

(def ?channel
[:map
[:type [:= :channel]]
[:emoji {:optional true} [:maybe ?emoji]]])

(def ?wallet
[:map
[:type [:= :wallet]]
Expand Down Expand Up @@ -60,6 +65,7 @@
[:catn
[:props
[:multi {:dispatch :type}
[:channel [:merge ?base ?channel]]
[:profile [:merge ?base ?profile]]
[:wallet [:merge ?base ?address-base ?wallet]]
[:saved-address [:merge ?base ?address-base ?saved-address]]
Expand Down
18 changes: 17 additions & 1 deletion src/quo/components/share/share_qr_code/view.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
(:require [clojure.set]
[oops.core :as oops]
[quo.components.avatars.account-avatar.view :as account-avatar]
[quo.components.avatars.channel-avatar.view :as channel-avatar]
[quo.components.avatars.user-avatar.view :as user-avatar]
[quo.components.avatars.wallet-user-avatar.view :as wallet-user-avatar]
[quo.components.buttons.button.view :as button]
Expand Down Expand Up @@ -131,8 +132,22 @@
{:size :size-32
:customization-color customization-color
:full-name full-name}]
:channel [channel-avatar/view
{:size :size-32
:emoji emoji
:locked-state :not-set
:customization-color customization-color
:full-name full-name}]
nil))

(defn- has-header?
[share-qr-type]
(case share-qr-type
(:wallet
:watched-address
:saved-address) true
false))

(defn- share-qr-code
[{:keys [share-qr-type qr-image-uri component-width customization-color full-name
profile-picture emoji on-share-press address]
Expand All @@ -153,7 +168,7 @@
{:color colors/white-opa-40
:container-style style/watched-account-icon}])]
[share-button {:on-press on-share-press}]]
(when (not= share-qr-type :profile)
(when (has-header? share-qr-type)
[header props])
[quo.theme/provider :light
[qr-code/view
Expand All @@ -163,6 +178,7 @@
:profile :profile
(:watched-address :wallet) :wallet-account
:saved-address :saved-address
:channel :channel
nil)
:customization-color customization-color
:full-name full-name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,20 @@
[utils.i18n :as i18n]
[utils.re-frame :as rf]))

(defn navigate-back
[]
(rf/dispatch [:navigate-back]))

(defn view
[]
(let [{:keys [url community-id]} (rf/sub [:get-screen-params])
(let [params (rf/sub [:get-screen-params])
;; NOTE(seanstrom): We need to store these screen params for when the modal closes
;; because the screen params will be cleared.
{:keys [url community-id]} @(rn/use-ref-atom params)
window-width (rf/sub [:dimensions/window-width])
{thumbnail-uri :logo
color :color
community-name :name} (rf/sub [:communities/for-context-tag community-id])
navigate-back (rn/use-callback #(rf/dispatch [:navigate-back]))
on-press-share (rn/use-callback
(fn []
(rf/dispatch
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
(ns status-im.contexts.communities.actions.share-community-channel.style
(:require [quo.foundations.colors :as colors]))

(defn container
[safe-area-top]
{:padding-top safe-area-top})

(def header-container
{:padding-horizontal 20
:padding-vertical 12})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,38 +8,41 @@
[utils.i18n :as i18n]
[utils.re-frame :as rf]))

(defn navigate-back [] (rf/dispatch [:navigate-back]))

(defn view
[]
(fn []
(let [{:keys [url chat-id]} (rf/sub [:get-screen-params])
(let [params (rf/sub [:get-screen-params])
;; NOTE(seanstrom): We need to store these screen params for when the modal closes
;; because the screen params will be cleared.
{:keys [url chat-id]} @(rn/use-ref-atom params)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the screen params are present in re-frame state, can't we just fetch it from there whenever we need it ?

Why create an extra copy? What's the constraint here ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The screen params are stored and then cleared from re-frame state when this view is animated to a closed state. So what will happen is that the url and chat-id will be set to nil, and this will cause a visual glitch with the QR code for a brief moment.

The above code is trying to prevent this visual glitch by keeping a local copy of the screen params. This way when the screen params are cleared while navigating, the QR code will still visualised with the intended url value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this apply only to this QR code (the visual glitch) or all QR codes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, I think most other QR codes are unaffected because we don't the screen-params for passing identifiers, they're sorta implicitly in scope for things like wallet accounts or contacts. But communities and community channels do not always have their identifiers implicitly in scope.

For example, we can navigate directly from the home screen to a community QR code, which leads to us passing the community-id through the screen params manually.

I've updated the PR to include a fix for community QR codes as well 🙏

{:keys [color emoji chat-name]} (rf/sub [:chats/community-channel-ui-details-by-id chat-id])
window-width (rf/sub [:dimensions/window-width])]
on-share-community-channel (rn/use-callback
#(rf/dispatch
[:communities/share-community-channel-url-with-data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a navigate back somewhere in this component. That can be extracted to the let bind and wrapped in a use-callback.

Not related to your code, but a simple housekeeping win.

chat-id])
[chat-id])]
[quo/overlay {:type :shell}
[rn/view
{:style {:padding-top (safe-area/get-top)}
{:style (style/container (safe-area/get-top))
:key :share-community}
[quo/page-nav
{:icon-name :i/close
:on-press #(rf/dispatch [:navigate-back])
:on-press navigate-back
:background :blur
:accessibility-label :top-bar}]
[quo/text-combinations
{:container-style style/header-container
:title (i18n/label :t/share-channel)}]
[rn/view {:style style/qr-code-wrapper}
[quo/gradient-cover
{:container-style
(style/gradient-cover-wrapper window-width)
:customization-color color}]
[rn/view
{:style {:padding-vertical 12}}
[qr-codes/qr-code
{:size (style/qr-code-size window-width)
:url url
:avatar :channel
:customization-color color
:emoji emoji
:full-name chat-name}]]]
[qr-codes/share-qr-code
{:type :channel
:qr-data url
:customization-color color
:emoji emoji
:full-name chat-name
:on-share-press on-share-community-channel}]]
[quo/text
{:size :paragraph-2
:weight :regular
Expand Down
3 changes: 2 additions & 1 deletion src/status_im/contexts/preview/quo/share/share_qr_code.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
:options [{:key :profile}
{:key :wallet}
{:key :saved-address}
{:key :watched-address}]}])
{:key :watched-address}
{:key :channel}]}])

(def possible-networks [:ethereum :optimism :arbitrum :myNet])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is :myNet 🤔


Expand Down