-
Notifications
You must be signed in to change notification settings - Fork 2
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/profile/edit profile screen UI and tests #108
Conversation
- Center GlideImage horizontally in EditProfileScreen - Adjust padding and layout for better UI alignment - Update text styles and field arrangements - Add rounded corners and borders to input fields - Ensure Save Changes button is centered and styled
…file/editProfileScreen-ui-and-tests
- Extract ProfileImage composable - Extract ProfileTextField composable - Extract ProfileSection composable - Add comments for better code understanding - Maintain existing UI layout and design
- Added UI tests for `EditProfileScreen` to verify UI elements and interactions. - Implemented field validation to check if name, date of birth, and description fields are filled.
…file/editProfileScreen-ui-and-tests
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.
Summary
Good initial implementation of the EditProfileScreen
. Improvements are needed for consistency and better alignment with the Figma design.
Important
Code Quality
- Enhance code consistency as noted in the comments.
Functionality
- Ensure the UI matches the Figma specifications more closely. Check other screens' code for consistency and inspiration.
Testing
- Review the existing tests to confirm they cover the updated UI and logic. Check other screens' tests for consistency, completeness, and inspiration.
Suggestions
- Address the changes highlighted in the code and file comments.
Steps before PR approved
- Implement the changes detailed in the code and file comments to ensure consistency and Figma alignment.
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
Hi @Harrish92! I just merged a PR (#102) that refactors every testTag in a separate file for better maintainability. Can you take a look at how it's done and apply it to your code? |
…file/editProfileScreen-ui-and-tests
- Revamped the tests for EditProfile to ensure better coverage and adherence to the testTag convention. - Improved the layout for better user experience and visual consistency. - Added a new component to the UI for enhanced functionality.
…file/editProfileScreen-ui-and-tests
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.
PR Review: EditProfileScreen
UI Implementation
Summary:
Overall, great work on implementing the EditProfileScreen
UI. The screen aligns well with the provided Figma designs and showcases attention to detail.
Review Notes:
- A few minor issues need addressing before approval.
- Refer to in-line code comments for specific suggestions and corrections.
Requested Changes:
- Address code consistency and formatting issues.
- Review and implement small optimizations as highlighted in the code comments.
Approval Status:
Pending – please make the suggested adjustments.
Good job so far! Let me know if you need further clarification on any points.
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Outdated
Show resolved
Hide resolved
…pdate description input field - Added tags for sectors to improve testability and organization. - Refactored tests for better adherence to conventions. - Changed the profile icon for a suitable look. - Updated the description input field to be more explicit.
@Harrish92
|
…file/editProfileScreen-ui-and-tests
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.
A few nitpicks to correct before PR approved for merge. We're almost there !! :)))
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Show resolved
Hide resolved
- Renamed test functions from saveValid... and saveInvalid... to editValid... and editInvalid... to better reflect their purpose. - Removed unnecessary toggled comments for cleaner code. - Ensured no extra spaces after test function declarations for better readability. - Reordered lines in the same order as the testTags. - Simplified validation logic for email.
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.
PR Review
Summary
Overall, a strong effort. The code mostly adheres to the standards and the Figma design, and the intended functionality is clear.
Important
Code Quality
The code is generally readable and consistent. However, there are a few areas where the naming conventions and spacing can be improved to better align with project guidelines.
Functionality
The feature works as expected, handling most scenarios effectively.
Testing
UI tests appear thorough and well-implemented.
Documentation
No changes are needed to external documentation.
Suggestions
Consider implementing the suggested naming convention changes for better readability and consistency.
Steps before PR Merge
Address the code naming changes as mentioned in the comments.
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileTest.kt
Outdated
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.
Summary
They are only small adjustments to fit the figma (please see comments).
Important
Code Quality
LGTM
Functionality
LGTM
Testing
LGTM please check when you change the code that it doesn't affect the tests.
Documentation
LGTM. There's a lot of comments which strongly contribute to the understanding of the code.
Steps before PR approved
Please read the comments to fit more to the figma
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/android/periodpals/ui/components/ProfileComponents.kt
Show resolved
Hide resolved
- Set `fillMaxWidth` to name, email, and dob for `OutlineFieldText`. - Removed the background color for the save button. - Removed the `HorizontalDivider`. - Resized profile image profile to 190.dp. - Renamed button to "Save".
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.
Please also remove the color of the button.
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.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.
LGTM now :)
Title
Description
This PR introduces the UI for
EditProfileScreen
, adds the option to change the profile image, and implements validation logic for the fields. It also includes tests for these functionalities. It closes issue #88.Changes
EditProfileScreen
.add_circle_icon
is clicked.Files
Added
app/src/androidTest/java/com/android/periodpals/ui/profile/EditProfileScreenTest.kt
Modified
app/src/main/java/com/android/periodpals/ui/profile/EditProfile.kt
Removed
Dependencies Added
Testing
EditProfileScreen
are correctly displayed.EditProfileScreen
pass successfully.Screenshots (if applicable)
EditProfileScreen
UI and tests #89