Skip to content
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

[bugfix/LIVE-11512]: return correct translation for user refused on device from swap-live-app #6665

Merged
merged 4 commits into from
Apr 22, 2024

Conversation

kallen-ledger
Copy link
Contributor

βœ… Checklist

  • npx changeset was attached.
  • Covered by automatic tests.
  • Impact of the changes:
    • ...

πŸ“ Description

return correct translation for user refused on device from swap-live-app

❓ Context

https://ledgerhq.atlassian.net/browse/LIVE-11512


🧐 Checklist for the PR Reviewers

  • The code aligns with the requirements described in the linked JIRA or GitHub issue.
  • The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
  • There are no undocumented trade-offs, technical debt, or maintainability issues.
  • The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
  • Any new dependencies have been justified and documented.
  • Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

@kallen-ledger kallen-ledger self-assigned this Apr 12, 2024
@kallen-ledger kallen-ledger requested a review from a team as a code owner April 12, 2024 11:21
Copy link

vercel bot commented Apr 12, 2024

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

5 Ignored Deployments
Name Status Preview Comments Updated (UTC)
ledger-live-docs ⬜️ Ignored (Inspect) Visit Preview Apr 18, 2024 11:03am
ledger-live-github-bot ⬜️ Ignored (Inspect) Visit Preview Apr 18, 2024 11:03am
native-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Apr 18, 2024 11:03am
react-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Apr 18, 2024 11:03am
web-tools ⬜️ Ignored (Inspect) Visit Preview Apr 18, 2024 11:03am

@live-github-bot live-github-bot bot added the desktop Has changes in LLD label Apr 12, 2024
@cng-ledger
Copy link
Contributor

cng-ledger commented Apr 17, 2024

@cng-ledger

Generating screenshots: workflow ended

Jobs status:
βœ… - generate-screenshots-linux

@kallen-ledger kallen-ledger requested review from a team as code owners April 17, 2024 09:37
@live-github-bot live-github-bot bot added the screenshots Screenshots have been updated label Apr 17, 2024
@@ -733,18 +738,23 @@ export const renderError = ({
withDescription?: boolean;
Icon?: (props: { color?: string | undefined; size?: number | undefined }) => JSX.Element;
}) => {
let tmpError = error;
Copy link
Contributor

Choose a reason for hiding this comment

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

May I know why ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's about the line 752 to update the error type to a specific type. But you can do this after the first 3 assertions, it will be more clear. If the first 3 catches do not catch the error, then you do a check (error instanceof CompleteExchangeError && error.message === 'User refused') to see whether the error type needs an update.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is correct, if we aren't doing any variable assignment at line 752, i can removed the let tmpError

@@ -733,18 +738,23 @@ export const renderError = ({
withDescription?: boolean;
Icon?: (props: { color?: string | undefined; size?: number | undefined }) => JSX.Element;
}) => {
let tmpError = error;
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is correct, if we aren't doing any variable assignment at line 752, i can removed the let tmpError

@cng-ledger cng-ledger merged commit 07d85e8 into develop Apr 22, 2024
32 of 33 checks passed
@cng-ledger cng-ledger deleted the bugfix/LIVE-11512 branch April 22, 2024 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop Has changes in LLD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants