-
Notifications
You must be signed in to change notification settings - Fork 1
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
User profile screen v2 #76
Conversation
I think the new UI design is awesome and perfectly addresses our past criticisms, amazing work 🎆 |
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.
The profilage page looks beautiful !
It is pleasant and intuitive to use and will provide a great user experience !
The code is also nicely structured which will facilitate for the integration of the viewmodel in the future 👍
I have just made a minor suggestion below.
Also, I've noticed that for naming the widget keys you use spaces to separate the words.
So I have looked up to other code for UI components and found out that we are using camelCase for the home view and snake_case for the login view.
This really minor because we only use the Key object in the tests but we may want to adopt a particular convention (I guess we can discuss that with the team in the next meeting).
Overall, the PR looks good to me 👍
Thank you for your work !
…fo cards are displayed
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 for the changes !
The addition of post and comment cards as well as shadows make the profile page looks even better 👍
I saw that you also added TODO comments at the places that will need to be modified when integrating the viewmodel, that's good and will make the integration easier.
I have just made little suggestions below (mostly typos).
Overall, very good job 🎉
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.
For some reason, not all my suggestions were showing up so I re-added them here (I think I forgot to add them to the review...)
Co-authored-by: Yoann Lafore <[email protected]>
Co-authored-by: Yoann Lafore <[email protected]>
Co-authored-by: Yoann Lafore <[email protected]>
Co-authored-by: Yoann Lafore <[email protected]>
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.
Good job, the design looks great! This PR looks good to me.
Co-authored-by: CHOOSEIT <[email protected]>
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.
LGTM
This pull request implements a new version of the user profile screen. After discussion with the team, we came to the conclusion that the version of the last sprint wasn't pleasant to use. Here are the main features brought by this PR:
A tab display for the personal posts and comments list : We realised that having two scrollable lists nested in another one resulted in a poor user experience. To simplify the navigation between posts and comments, we added a tab bar that switches from one list to the other. The navigation on the screen is thus more intuitive.
General design rework : this new version tries to showcase a more minimalistic design. It is based on the mockup of the home page, so that it respects the general style of the app.
Test file : more test were added for this new version as the new tab navigation needs to be tested. The tests check that the widgets are displayed and ensure that the navigation between the two columns (posts/comments) is working fine.