-
Notifications
You must be signed in to change notification settings - Fork 110
backend/frontend: gate send-to-self dropdown on firmware support #3625
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
base: staging-send-to-self-dropdown
Are you sure you want to change the base?
backend/frontend: gate send-to-self dropdown on firmware support #3625
Conversation
| alertUser(t('device.firmwareUpgradeRequired')); | ||
| return; | ||
| } | ||
| } |
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.
in Pocket we use this
bitbox-wallet-app/frontends/web/src/routes/market/pocket.tsx
Lines 385 to 405 in 91c8611
| <Dialog title={t('upgradeFirmware.title')} open={fwRequiredDialog}> | |
| {t('device.firmwareUpgradeRequired')} | |
| <DialogButtons> | |
| <Button | |
| primary | |
| onClick={() => { | |
| setFirmwareUpdateDialogOpen(true); | |
| navigate(`/settings/device-settings/${deviceIDs[0] as string}`); | |
| }}> | |
| {t('upgradeFirmware.button')} | |
| </Button> | |
| <Button | |
| secondary | |
| onClick={() => { | |
| setFwRequiredDialog(false); | |
| navigate(-1); | |
| }}> | |
| {t('dialog.cancel')} | |
| </Button> | |
| </DialogButtons> | |
| </Dialog> |
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.
Nice. Will ask my agent to extract this into a resuable component later 😁
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.
@thisconnect done, PTAL.
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.
Tested with 9.18.0 and tried to find why it didn't show. But reading the code the FW upgrade dialog should only shows when handleSend is called. Could you change to it shows earlier?
I suggest either when the user clicks the dropdown or after choosing an option from the dropdown.
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.
Agree - I think it'd be nice to show it earlier, before the user presses the send btn
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.
backend utACK with a minor comment (and I think you probably need to regenerate the mock file)
backend/handlers/handlers.go
Outdated
| handlers.log.WithField("requested", rootFingerprintHex).Warn("features requested for non-connected keystore") | ||
| return response{ | ||
| Success: false, | ||
| ErrorMessage: "keystore not connected", |
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.
[nit] maybe something clearer that shows the difference between this case and the one where connectedKeystore is nil?
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.
Done
e4af881 to
75dc3c3
Compare
8ca7859 to
0fdee13
Compare
No description provided.