-
Notifications
You must be signed in to change notification settings - Fork 275
Jon/fix/fiat text #5785
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
Jon/fix/fiat text #5785
Conversation
8c5212a to
928a17f
Compare
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| title: { title: lstrings.quote_fixed_quotes, rightTitle }, | ||
| data: [...quotes.filter((quote: EdgeSwapQuote) => !quote.isEstimate)] | ||
| data: [ | ||
| ...quotes.filter((quote: EdgeSwapQuote) => !quote.isEstimate), | ||
| ...quotes.filter((quote: EdgeSwapQuote) => !quote.isEstimate), | ||
| ...quotes.filter((quote: EdgeSwapQuote) => !quote.isEstimate) | ||
| ] |
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.
Avoid triplicating fixed quotes in the provider list
The sectionList for the swap confirmation modal now spreads the array of non‑estimate quotes three times (...quotes.filter(...) repeated on lines 142‑144). This causes every fixed quote to appear three times in the "fixed quotes" section and inflates the data set, which will mislead users and can slow the list rendering. The previous implementation included the array only once; please revert to a single spread when building the section data.
Useful? React with 👍 / 👎.
928a17f to
903c969
Compare
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.
The AI seems to think there is a problem, but I don't see it. If you can just double-check that, I certainly have no objection.
| pluginId: wallet.currencyConfig.currencyInfo.pluginId, | ||
| maxPrecision: 2, | ||
| subCentTruncation: true | ||
| }) |
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.
Is this AI comment correct?
828c26a to
afa7272
Compare
afa7272 to
e97a897
Compare
A component that returns a node containing plain text is misleading. - Callers may try to differentiate between strings and `ReactNodes` and `FiatText's` return value can cause runtime errors about strings not in `<Text>` components. - Update so the `useFiatText` hook is used for getting the raw text value, while the `FiatText` is a convenience component that returns an `EdgeText` wrapped component
e97a897 to
2401f92
Compare
| pluginId: wallet.currencyConfig.currencyInfo.pluginId, | ||
| cryptoExchangeMultiplier: denomination.multiplier, | ||
| isoFiatCurrencyCode | ||
| }) |
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.
Bug: Inconsistent Denomination Handling in useFiatText
The useFiatText hook's cryptoExchangeMultiplier parameter is sourced inconsistently. It's sometimes taken from component props (CryptoFiatAmountTile) or mismatched with the nativeCryptoAmount source (ExchangeRate2). This deviates from the original FiatText component's consistent denomination handling, potentially causing incorrect fiat conversions when display and exchange denominations differ.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have:
Note
Standardizes
FiatTextto render anEdgeTextand refactors components to useuseFiatText/useTokenDisplayData, simplifying props and fixing nested text issues across UI.FiatTextnow returns anEdgeText(adds optionalstyle), instead of a raw string.useFiatText+useTokenDisplayDatato precompute fiat strings.ExchangeRate2: replace inlineFiatTextwith precomputedfiatTextstring and updated hooks.FlipInputModal2: precomputebalanceFiatTextandfeeFiatTextvia hooks; replace inlineFiatTextinstances.IconDataRow: props acceptstring | ReactNodeand centralized render helper.CryptoFiatAmountRow,CryptoFiatAmountTile,FiatAmountTile: typings toReact.FC, conditional rendering to avoid double-wrapping, use precomputed fiat where applicable.RequestScene: passstyledirectly toFiatTextand remove extraEdgeTextwrapper.Loans/LoanCreateConfirmationScene: simplify by renderingFiatTextdirectly.Textwrapping around fiat values (e.g.,€1.83,€0.000000).Written by Cursor Bugbot for commit 2401f92. This will update automatically on new commits. Configure here.