Skip to content

Conversation

@benma
Copy link
Contributor

@benma benma commented Oct 16, 2025

No description provided.

@benma benma requested review from bznein and shonsirsha October 16, 2025 15:05
alertUser(t('device.firmwareUpgradeRequired'));
return;
}
}
Copy link
Collaborator

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

<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>

Copy link
Contributor Author

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 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thisconnect done, PTAL.

Copy link
Collaborator

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.

Copy link
Collaborator

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

Copy link
Collaborator

@bznein bznein left a 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)

handlers.log.WithField("requested", rootFingerprintHex).Warn("features requested for non-connected keystore")
return response{
Success: false,
ErrorMessage: "keystore not connected",
Copy link
Collaborator

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@benma benma force-pushed the send-to-self-version-check branch from e4af881 to 75dc3c3 Compare October 16, 2025 15:24
@benma benma force-pushed the send-to-self-version-check branch from 8ca7859 to 0fdee13 Compare October 16, 2025 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants