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.
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
feat: magic code input (passwordless login) #16076
feat: magic code input (passwordless login) #16076
Changes from all commits
29bf8b9
0bafd04
20fe253
80483a2
94ce197
ef05927
4911091
968f54b
5471423
42ac6c8
387cd96
67710ca
f0f749c
ce9d29f
5ccde1c
423393a
592e1eb
469f5e4
501507c
a1f6cfd
85a3cdf
7c06175
b24e9e9
f1e58c2
f29e6e1
7d353c3
d8e6314
83971d2
3e562f8
9c0749a
4e4e220
3ddb740
b1a39eb
01be1c4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Clearing input has caused this regression
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.
Same comment here about naming
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.
Methods should never be named for what they handle, they should only be named for what they do. This is part of the checklist:
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.
My bad, I must have skipped it when I read it. There's an issue regarding MagicCodeInput yet - #18325 - where this can be fixed, if I end up assigned, I can do it there.
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.
I don't like this style definition
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.
Actually why can't we use TextInput instead of manual Text?
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.
This is necessary to hide the input above the text, so that we can keep that behaviour of copy paste in each input as well as focus on tap.
What exactly don't you like abou the style definition? Do you want me to move it to a style object?
This is something I had previously, but it was too problematic. You would need to change the focus to the input above and force it to never focus, pass a
focused
prop (which was another prop needed to add to the BaseTextInput only to show the focused state), be non editable. With Text it becomes simpler, it's only a wrapper for a number with some style.