Swaps token sources/verification messaging update#10346
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
d74f1e0 to
e1fc233
Compare
Builds ready [e1fc233]
Page Load Metrics (624 ± 34 ms)
|
|
How does this handle the case where token metadata differs between the sources used by the API? We talked about doing something like this before, and the problem was that the API didn't expose whether the metadata differed between sources at all, which is much more important than whether it was confirmed by multiple sources. |
app/_locales/en/messages.json
Outdated
| "description": "Used in the transaction display list to describe a swap. $1 and $2 are the symbols of tokens in involved in a swap." | ||
| }, | ||
| "swapTokenVerificationMessage": { | ||
| "message": "$1 Always confirm the token address on $2. $3", |
There was a problem hiding this comment.
$1 here seems like an entirely separate sentence. Could this be broken out of this message, to simplify the work for translators? I'm not entirely sure why it's embedded.
There was a problem hiding this comment.
Similarly, $2 and $3 can be represented as a single placeholder. There is no need for them to be separated.
e1fc233 to
9910d37
Compare
|
@Gudahtt Your comments have been addressed in the latest commit |
9910d37 to
91effb4
Compare
Builds ready [91effb4]
Page Load Metrics (919 ± 52 ms)
|
Builds ready [bb2e461]
Page Load Metrics (923 ± 30 ms)
|
|
Needs a rebase to fix conflicts from the semicolon eslint change |
bb2e461 to
3aa2eb7
Compare
|
This has been rebased |
Builds ready [3aa2eb7]
Page Load Metrics (683 ± 105 ms)
|
| <div className="actionable-message__message">{message}</div> | ||
| {(primaryAction || secondaryAction) && ( | ||
| <div className="actionable-message__actions"> | ||
| <div className={classnames('actionable-message__actions')}> |
There was a problem hiding this comment.
Nit: This change is not necessary
…onable message component
Builds ready [a039542]
Page Load Metrics (652 ± 75 ms)
|
|
|
||
| .actionable-message__action { | ||
| font-weight: normal; | ||
| cursor: pointer; |
There was a problem hiding this comment.
Nit: This rule is redundant. It already has cursor: pointer from the base actionable-message__action class
There was a problem hiding this comment.
This can get cleaned up in the PR that makes this more accessible.
Gudahtt
left a comment
There was a problem hiding this comment.
LGTM!
The only problem I found is that the Continue button is not accessible. We should be using a <button> element there instead of making a div clickable. I won't block on it because it's a pre-existing issue with ActionableMessage buttons, but we should try to fix this before this ships.
Provides a more emphasized message about token sources and verification of tokens on etherscan, on the build quote screen of swaps. In addition, if our server indicates that the token is only recognize by 1 of the multiple sources that it tracks, then it requires the user to click "Continue" before they can request quotes.
Resolves #10325
Peek.2021-02-02.18-00.mp4