V6 fix initial state upi intent #2921
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 7272a8c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Size Change: +8 B (0%) Total Size: 755 kB
ℹ️ View Unchanged
|
size-limit report 📦
|
|
longyulongyu
requested review from
marcperez,
pabloai,
sponglord,
m1aw and
ribeiroguilherme
as code owners
sponglord
reviewed
Oct 28, 2024
| @@ -34,7 +34,7 @@ export default function UPIComponent({ defaultMode, onChange, onUpdateMode, payB | |||
| const { i18n } = useCoreContext(); | |||
| const getImage = useImage(); | |||
| const [status, setStatus] = useState<UIElementStatus>('ready'); | |||
| const [isValid, setIsValid] = useState<boolean>(true); | |||
| const [isValid, setIsValid] = useState<boolean>(defaultMode === 'qrCode'); | |||
There was a problem hiding this comment.
How come the v5 version has a constant (defaultMode === UpiMode.QrCode) but here we're using a string?
There was a problem hiding this comment.
In v6, we use string literals for the UpiMode, but in v5 we define it as enum, that's why we need to code differently. The reason to change the type definition is because string literals are easier for JS users.
There was a problem hiding this comment.
Just an observation: we use the string qrCode in quite a few places - it just feels like something that could be defined as a constant somewhere.
This is also true in a lot of other places in our codebase
There was a problem hiding this comment.
Just an observation: we use the string
qrCodein quite a few places - it just feels like something that could be defined as a constant somewhere. This is also true in a lot of other places in our codebase
It feels a bit too much for this field, as it only allows 3 values and there is already a type guard for it. Typing any other strings throws a Typescript error.
Otherwise we will need to define the UpiMode like this, so that we can use VPA, QR_CODE or INTENT in the code.
export const VPA = 'vpa';
export const QR_CODE = 'qrCode';
export const INTENT = 'intent';
export type UpiMode = typeof VPA | typeof QR_CODE | typeof INTENT;
sponglord
approved these changes
Oct 28, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
cherry pick from fe369df