-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Merge TextInputWithPrefix into TextInput #7937
Conversation
@shawnborton Could you please review the design here? |
I think it looks pretty good, but there is a faint highlight of the second input. Can we get rid of that? Could we also slightly reduce the gap between the |
@shawnborton All settled now. But did you notice the save button? |
Nice, looks great to me! |
Should the save button be the same height as the input box? |
I agree with you Puneet but I think that might be a separate issue from this one? |
Ah ok got it! @parasharrajat let's make sure to report that. |
Or if it's just this one button, let's just fix it as part of this PR and add $250 extra for it. |
Updated |
PR Reviewer Checklist
|
@parasharrajat could you help me with navigating to the room's settings page? I'm getting this error on
|
This is not caused by my changes. I think refreshing the page should fix this one. |
Yep I tried clean build, hard refresh, clean local storage etc. |
You can open the old rooms' setting page to test it. |
Thanks, but that didn't help. I'll ask about this on slack |
@parasharrajat I'm not able to modify the room name in |
Any steps? |
Sorry, my bad. I was testing the default rooms (#announce and #admin) which aren't editable. |
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.
@puneetlath LGTM! 🎉️
The last PR is ready. Waiting for this one to be merged. |
@puneetlath before you leave, Could you please review this? Thanks. |
|
||
this.state = { | ||
isFocused: false, | ||
labelTranslateY: new Animated.Value(activeLabel ? styleConst.ACTIVE_LABEL_TRANSLATE_Y : styleConst.INACTIVE_LABEL_TRANSLATE_Y), | ||
labelScale: new Animated.Value(activeLabel ? styleConst.ACTIVE_LABEL_SCALE : styleConst.INACTIVE_LABEL_SCALE), | ||
passwordHidden: props.secureTextEntry, | ||
textInputWidth: 0, | ||
prefixWidth: 20, |
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.
Should this be a constant?
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.
Hmm, actually 0
should be fine here.
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.
@parasharrajat why not 0
?
When the prefix is of more than one character, width 20
will be wrong.
And we're calculating the correct width onLayout
anyway
@@ -404,6 +404,16 @@ function parseStyleAsArray(styleParam) { | |||
return _.isArray(styleParam) ? styleParam : [styleParam]; | |||
} | |||
|
|||
/** |
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.
Can we add a description to this function as to why it exists. It's pretty unclear what the purpose of this is on its own.
@puneetlath @rushatgabhane Updated. |
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.
@puneetlath LGTM!
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by @puneetlath in version: 1.1.42-0 🚀
|
🚀 Deployed to production by @chiragsalian in version: 1.1.42-6 🚀
|
Details
Fixed Issues
$ #6584
$ #7308
Tests
PR Review Checklist
Contributor (PR Author) Checklist
main
before submitting my PR for review### Fixed Issues
section abovesrc/languages/*
files (if applicable)Styling.md
) for all style edits I madePR Reviewer Checklist
main
before submitting the PR### Fixed Issues
section abovesrc/languages/*
files (if applicable)QA Steps
Tested On
Screenshots
Web | Desktop
Mobile Web
iOS
Android