Full Sweep: Drop the USE_UPDATED_SIGNING_UI feature flag #3475
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
In particular, the data signature detail components had an `excludeHeader` prop used for the old flow. This prop is now gone, and invoking components have been converted to our standard function declaration style. Also nuked two *DetailPanel components that were no longer in use and were consuming the old API.
This is no longer needed now that the updated signing UI is the only signing UI.
The new signing UI flow gives us some guarantees about things being set when these pages are rendered, and handles certain things being unset internally. This lets us drop some top-level code and lean on the Signing component for most of the work.
In particular, a lot of Ledger stuff that was potentially-undefined in the past is now known to be set because it gets called from code flows that have already specialized by the time it gets called.
Eventually this can probably resolve through custom networks as well, but this is a reasonable start vs having a one-off pseudo-map with only Ethereum covered.
| const { t } = useTranslation("translation", { keyPrefix: "signing.EIP4361" }) | ||
|
|
||
| const chainName = | ||
| NETWORK_BY_CHAIN_ID[signingData.chainId.toString()]?.name ?? "Unknown" |
There was a problem hiding this comment.
Would be nice to add "unknown" to translations
There was a problem hiding this comment.
Yeah agreed… Could even use signTransaction.unknownNetwork tbh, but the PR already started to get away from me a little 😬
| ) : ( | ||
| <> | ||
| <div className="label header"> | ||
| {internal ? t("signatureRequired") : t("dappSignatureRequest")} |
There was a problem hiding this comment.
We can remove dappSignatureRequest form messages.json. It is no longer used.
jagodarybacka
approved these changes
Jun 19, 2023
Merged
jagodarybacka
added a commit
that referenced
this pull request
Jun 23, 2023
## What's Changed * Repeater Connection: Forward requests to new default when default is switched off during dApp connection flow by @Shadowfiend in #3462 * Auto-Not-So-Matic: Fix two matic.network URLs to polygon.technology by @Shadowfiend in #3483 * v0.38.0 by @kkosiorowska in #3480 * Case Dismissed: Forcibly show DAppConnectionInfoBar popover on first dApp connection by @Shadowfiend in #3464 * Add Hardhat Fork Functionality by @0xDaedalus in #3247 * Faded Jeans: Rename fadeIn class to fade_in by @Shadowfiend in #3485 * Fix issue for discovery transaction hash by @kkosiorowska in #3458 * Full Sweep: Drop the USE_UPDATED_SIGNING_UI feature flag by @Shadowfiend in #3475 * Token Discovery - Remap redux asset balances by @hyphenized in #3195 * Make specific warnings for adding custom asset by @kkosiorowska in #3478 * Run NFTs e2e tests on a controlled wallet address by @michalinacienciala in #3487 * v0.38.1 by @jagodarybacka in #3484 **Full Changelog**: v0.38.1...v0.39.0 Latest build: [extension-builds-3496](https://github.com/tahowallet/extension/suites/13792957673/artifacts/764738599) (as of Thu, 22 Jun 2023 14:27:32 GMT).
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.
This takes care of the remaining cleanup from the old
signing flow:
And of course the removal of the feature flag itself.
Along the way I also fixed a couple of components that
were using
React.FCandexport constin a way thatwe don't do anywhere else.
Testing
correctly for Ledger and non-Ledger flows :)
Closes #3449 .
Latest build: extension-builds-3475 (as of Mon, 19 Jun 2023 09:38:28 GMT).