-
Notifications
You must be signed in to change notification settings - Fork 984
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
[Fixes #8066] Added native ENS registration #8317
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (90)
|
2b886fa
to
5979cae
Compare
@@ -2149,3 +2152,119 @@ | |||
:shake-event | |||
(fn [cofx _] | |||
(logging/show-logs-dialog cofx))) | |||
|
|||
(re-frame/reg-fx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this namespace is only for register-handler-fx
, and they should only contain simple events that call a function.
this would belong to ens.core
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/status_im/events.cljs
Outdated
(fn [[registry name cb]] | ||
(ens/get-addr registry name cb))) | ||
|
||
(defn- on-resolve [registry custom-domain? username address public-key s] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
src/status_im/events.cljs
Outdated
(stateofus/valid-username? username))) | ||
|
||
(handlers/register-handler-fx | ||
:ens/set-state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that if you use defn/fx to define the functions used in these events you can avoid declaring them here and simply use the attribute map key like this:
(fx/defn set-state
{:events [:ens/set-state]}
[{:keys [db]} state]
{:db (assoc-state db state)})
src/status_im/subs.cljs
Outdated
(re-frame/reg-sub | ||
:account/usernames | ||
:<- [:account/account] | ||
(fn [acc] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use account
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
(str "https://etherscan.io/address/" address)) | ||
|
||
(views/defview terms [] | ||
(views/letsubs [{:keys [contract]} [:get-screen-params :ens-terms]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this in get-screen-params here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's how we pass arguments to screens?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since @flexsurfer subs PR we stop using any kind of getter subscriptions, just create a simple sub like :ens/terms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i believe for screens it works a little bit different, btw if this is a current screen you can use[:get-screen-params]
and it will return data for current screen
:else :main-icons/username)) | ||
|
||
(defn- icon-wrapper [color icon] | ||
[react/view {:style {:margin-right 10 :width 32 :height 32 :border-radius 25 :align-items :center :justify-content :center |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be defined as a style separately or at least not in 1 line so stayl below char limit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe @flexsurfer is in favor of having inline styles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am too but just talking about the layout here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we still don't have any agreement on this, imo it's about balance if it's too noisy it can be moved to styles for sure
@asemiankevich Can you detail more 7. ? Do you always experience that, or only under some condition? |
Steps to reproduce:
Actual result: confirmation screen shown that name was added No special conditions here i think. |
Ah ok thanks! |
|
@jeluard can we do something about that top toolbar sitting on top of the OS Status bar? there's no way we can release it with that stuff going on, it should account for the height of status bar in top padding. |
ENS usernames intro screen,
Username entry screen:
Username checkout screen:
ENS settings
|
Expected: https://www.figma.com/file/TNCyHKtR3sx5EL6YznFWUa4O/Profile?node-id=1822%3A945 |
@asemiankevich Please provide exact steps to reproduce the issue |
i am sorry, @jeluard this happens when you:
|
@errorists @Serhy @asemiankevich All reported issues should be fixed now |
@jeluard i found some issues on android, please take a look.
|
@asemiankevich thanks, looked at the copy, it looks the same to me as with the Figma? The correct string revised by Rachel is 'Agree to Terms of name registration. I understand that my wallet address will be publicly connected to my username. |
@asemiankevich I fixed the first issue. I think we can proceed and merge now. |
100% of end-end tests have passed
Passed tests (47)Click to expand |
Signed-off-by: Andrey Shovkoplyas <motor4ik@gmail.com>
fixes #8066
Summary
Implement ENS registration according to latest Figma designs.
Note to testers Local names persistency is not part of this PR. Thus initial welcome screen is always the same even if you already registered some names using native registration.
Address and public key will be connected to names, and stateofus names can be be registered.
(superseed #8248)
Test
All screens and registration implemented
Names are persisted
Pixel perfect
Name removal is not part of this PR
status: READY