-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
fix: LoginPage for wide screens #6137
Conversation
<Form style={[ | ||
styles.flex1, | ||
styles.alignSelfStretch, | ||
props.isSmallScreenWidth && styles.signInPageNarrowContentContainer, | ||
props.isSmallScreenWidth ? styles.ph5 : styles.ph4, | ||
]} | ||
> |
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.
Indentation looks a little weird here. Maybe ]}>
?
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, it's totally fine. There are two ways here.
This one
e.g.
App/src/pages/home/report/ReportActionCompose.js
Lines 499 to 506 in 5362f01
<View style={[ | |
(this.state.isFocused || this.state.isDraggingOver) | |
? styles.chatItemComposeBoxFocusedColor | |
: styles.chatItemComposeBoxColor, | |
styles.chatItemComposeBox, | |
styles.flexRow, | |
]} | |
> |
and
<Form
style={[
styles.flex1,
styles.alignSelfStretch,
props.isSmallScreenWidth && styles.signInPageNarrowContentContainer,
props.isSmallScreenWidth ? styles.ph5 : styles.ph4,
]}
>
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.
Huh, Ok well that's fine with me. This was a NAB.
return content; | ||
} | ||
|
||
return <SignInPageWideContainer>{content}</SignInPageWideContainer>; |
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 there really an advantage to having the SignInPageWideContainer
be its own component? I think it'd be better for us to get rid of the SignInPageWideContainer
altogether and have the View and SVG components it contains live here in this file instead. In other words, why have these as two files when it's not really complicated to have them be one?
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.
It looks clean this way but let me know I can merge.
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 just merged. Thanks for the suggestion.
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.
Thanks, I appreciate your willingness to do that. 👍
✋ 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 production by @Julesssss in version: 1.1.21-1 🚀
|
Details
Use KeyboardAvoidingView to push the input into the viewport and Scrollview to Allow the user to scroll the screen.
Fixed Issues
$ #5159
Tests
QA Steps
Tested On
Screenshots
Web | Desktop
Mobile Web
iOS
Android
output_file.mp4