Connected dApps: a list with empty state, plus button, etc etc / fetching dApps / disconnecting dApps #19943
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (59)
|
alwx
force-pushed
the
dapp-list
branch
3 times, most recently
from
184da9b
to
a3553a3
Compare
alwx
changed the title
|
Now it also includes the disconnect flow and includes fetching of dApps from the WalletConnect API. Simulator.Screen.Recording.-.iPhone.15.-.2024-05-23.at.15.11.44.mp4 |
clauxx
reviewed
May 23, 2024
src/status_im/contexts/chat/messenger/composer/mentions/view.cljs
Outdated
Show resolved
Hide resolved
Comment on lines
+10
to
+12
| [{:keys [wallet-account dapp on-disconnect]}] | ||
| (let [{:keys [avatar name]} dapp | ||
| {:keys [color]} wallet-account] |
There was a problem hiding this comment.
to improve performance, might be better to pass only the props it needs (avatar, name, color, on-disconnect) instead of the entire maps
| :style (style/header-text (when subtitle true))} | ||
| title]]) | ||
| [{:keys [title wallet-account on-close on-add]}] | ||
| (let [{:keys [color]} wallet-account] |
There was a problem hiding this comment.
same here, if only the color is needed, then we should just pass it instead of passing the whole account map
Comment on lines
+101
to
+102
| [rn/flat-list | ||
| {:data pairings |
There was a problem hiding this comment.
I wonder if we should normalize the pairings data in the db, so that we have in the db a list of pairing topics and a map with topics->pairing-data. This way we would only pass the id to the flatlist item, and the item component can retrieve the pairing data with a subscription. Otherwise, we always have to map over the pairings vector to find the relevant one, which might cause perf issues in the future.
| {:db (assoc db :wallet-connect/web3-wallet web3-wallet) | ||
| :fx [[:dispatch [:wallet-connect/register-event-listeners]]]})) | ||
| {:db (assoc-in db [:wallet :wallet-connect :web3-wallet] web3-wallet) |
There was a problem hiding this comment.
I think we should keep the :wallet-connect/... keys as roots and avoid nesting unless necessary. I remember there was a conversation at the offsite about this topic @J-Son89 @cammellos
There was a problem hiding this comment.
my point here is that it becomes pretty much a mess but interesting to hear other opinions
There was a problem hiding this comment.
Maybe we can have a session to discuss the db structure next week, but for now I'd propose to keep the structure with root keys, otherwise there's going to be some nasty conflicts. WDYT?
Comment on lines
+38
to
+40
| :on-press (fn [] | ||
| (rf/dispatch [:navigate-to | ||
| :screen/wallet.connected-dapps]))}) |
There was a problem hiding this comment.
the fn can go outside the component
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #19839
Fixes #19840
Summary
Simulator.Screen.Recording.-.iPhone.13.-.2024-05-16.at.17.11.53.mp4
Platforms
Areas that maybe impacted
Functional
status: ready