-
Notifications
You must be signed in to change notification settings - Fork 0
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
Login Screen - Information about the app #293
Login Screen - Information about the app #293
Conversation
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.
Everything looks fantastic, I love it! I added 2 comments on small changes that would be really nice to have a look at. Otherwise the pager is implemented very neatly and could be reused in other parts of the app I guess.
app/src/main/java/com/lastaoutdoor/lasta/ui/screen/login/components/LoginContent.kt
Show resolved
Hide resolved
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.
Hello Jérémy, good job improving the app's aspect and the user experience :)
I requested small changes so that you modularizes your code a bit more and change a small nitpick in translations, but in terms of functionality your code seems perfect.
app/src/main/java/com/lastaoutdoor/lasta/ui/screen/login/components/LoginContent.kt
Show resolved
Hide resolved
app/src/main/java/com/lastaoutdoor/lasta/ui/screen/login/components/LoginContent.kt
Outdated
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
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.
Hi Jerem,
your PR is really aesthetic, it's feeling super smooth, looking better than ever after the requested changes, and it is a great new feature to our app, crucial for new users. I really like that you tested 100%, nice effort !
You also added comments which is great. Simple yet solid PR, nice job :)
I'm approving, but as a suggestion, you're not forced to change this because it is subjective to what I think personally, I was thinking that "Swipe to learn more..." is not the best way to formulate this first sentence, and I would put something like "New to Lasta ? Swipe to learn more !", "What is Lasta ? Swipe for more info ->", or something similar to this. Let me know what you think about it, and good job once more !
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.
Well done my Lord, it's perfect
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.
Everything in order on my side! Nice addition and insane testing :)
Hi Cassio, thanks for your feedback ! This is indeed something that we could discuss all together at the next meeting and can easily be changed later on. For now I think that my version is OK, but if needed we will change that later this week |
Login Screen - Information about the app
Content
I added a pager in login screen where basic information about the app can be found. All text are translated and all new composable have been tested.
If you want to suggest other content in those pages, you are more than welcome to request changes.
Preview
Dark theme
Normal theme