Feature: Ledger Wallet Support for Signing Messages #562
Conversation
There was a problem hiding this comment.
Hello @blouflashdb,
thank you very much for this PR. The functionality is already mostly there 🔥
However, there are some things that need to be addressed. I will take it from here and work on these changes in the new year.
I wish you a happy new year 🍾
| msgid "Sign Message" | ||
| msgstr "Mensahe sa Pag-sign" | ||
|
|
||
| #: src/views/SignMessageLedger.vue:17 | ||
| msgid "Signer" | ||
| msgstr "Signer" |
There was a problem hiding this comment.
Missing translations should be left empty, otherwise, our translators don't know that they'll need to work on those.
Also, missing translations automatically fall back to English, so no English fallback needs to be provided manually.
| case WalletType.LEDGER: | ||
| this.$router.push({name: `${RequestType.SIGN_MESSAGE}-ledger`}); | ||
| return; |
There was a problem hiding this comment.
Have to ensure the active account is set to the signer address, even when the address was not selected by the user but requested via the API. Otherwise, SignTransactionLedger will sign with the wrong address.
| <div class="bottom-container"> | ||
| <LedgerUi small></LedgerUi> | ||
| </div> |
| <Identicon :address="activeAccount.userFriendlyAddress"></Identicon> | ||
| </div> | ||
| <div class="account-text"> | ||
| <div class="label" id="signer-label">{{ activeAccount.defaultLabel }}</div> |
There was a problem hiding this comment.
Should show the user label instead of the default label.
| import { WalletInfo } from '@/lib/WalletInfo'; | ||
| import { AccountInfo } from '@/lib/AccountInfo'; | ||
| import LedgerApi from '@nimiq/ledger-api'; |
| .small-page#sign-message .page-body { | ||
| display: flex; | ||
| flex-direction: column; | ||
| padding-left: 1rem; | ||
| padding-right: 1rem; | ||
| padding-bottom: 0; | ||
| } | ||
|
|
||
| .account { | ||
| display: flex; | ||
| flex-direction: row; | ||
| align-items: center; | ||
| padding: 3rem 2rem; | ||
| } | ||
|
|
||
| .account .identicon { | ||
| width: 5.625rem; | ||
| height: 5.625rem; | ||
| margin-right: 2rem; | ||
| flex-shrink: 0; | ||
| } | ||
|
|
||
| .account-text { | ||
| display: flex; | ||
| flex-direction: row; | ||
| justify-content: space-between; | ||
| flex-grow: 1; | ||
| font-size: 2rem; | ||
| max-width: calc(100% - 5.625rem - 2rem); | ||
| /* .identicon width and margin-right */ | ||
| } | ||
|
|
||
| .account .label { | ||
| font-weight: 600; | ||
| overflow: hidden; | ||
| text-overflow: ellipsis; | ||
| white-space: nowrap; | ||
| } | ||
|
|
||
| .account .label-right { | ||
| opacity: 0.7; | ||
| margin-left: 2rem; | ||
| } | ||
|
|
||
| #message { | ||
| font-size: 1.75rem; | ||
| line-height: 1.2; | ||
| color: rgba(31, 35, 72, .7); | ||
| /* Based on Nimiq Blue */ | ||
| background-color: rgba(31, 35, 72, .05); | ||
| /* Based on Nimiq Blue */ | ||
| border: solid 1px rgba(31, 35, 72, .1); | ||
| /* Based on Nimiq Blue */ | ||
| border-radius: 0.5rem; | ||
| outline: none; | ||
| width: 100%; | ||
| padding: 1.5rem; | ||
| flex-grow: 1; | ||
| resize: none; | ||
| font-family: 'Fira Mono', monospace; | ||
| } | ||
|
|
||
| .bottom-container { | ||
| height: 23rem; | ||
| } |
There was a problem hiding this comment.
Some styling issues regarding sizes and paddings..
| </PageHeader> | ||
|
|
||
| <PageBody> | ||
| <textarea id="message" readonly="readonly" :value="request.message"></textarea> |
There was a problem hiding this comment.
Would be theoretically better to use a class instead of an id here, as if there were to be a second SignMessageLedger instance on the page, it would be a duplicate identifier. Not that it's an actual issue here though, as there won't be two SignMessageLedger instances at the same time :)
Feature: Ledger Wallet Support for Signing Messages
This pull request introduces the ability to use Ledger Wallets to sign messages.
Key Notes
1. StatusMessage Component
StatusMessagecomponent to work as expected.StatusMessagefrom this PR.2. Tab Space UI Feature
Focus
The primary goal of this PR is to enable message signing with Ledger Wallets, which I believe is the most critical functionality at this time.
Please review and let me know if there are additional adjustments needed!