-
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
Dev/additional settings #323
base: main
Are you sure you want to change the base?
Conversation
checkbox for terms and button disabled
AI-Generated Summary: This pull request adds display features for first name, last name, and phone number fields in the user profile section. Furthermore, it includes an image cropping functionality for the profile picture. Changes have been made in the 'package-lock.json', 'package.json', and 'MyAccount.tsx' files, with the latter being within the 'Profile' folder. Additional packages such as 'ImgCrop' from 'antd-img-crop' have been included. The language translation files for English, French, and German have been updated to add translations for 'First Name' and 'Last Name'. The package-lock.json file has been updated with new dependencies, their versions and hashes. |
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.
Looks good overall, marking as "Request changes" because of the question about splitting the names in our database already.
@@ -21,17 +23,37 @@ const MyAccount: React.FC = () => { | |||
|
|||
useEffect(() => { | |||
if (user) { | |||
setName(user.displayName); | |||
if (user.displayName) { |
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.
If I see this correctly, you're dealing with the names stored in the previous format (before the first/last name split) by splitting them on the fly.
Would it make sense to "bake" this process into our database? Essentially going through all names we currently have, split them and store them correctly? Tagging @hugo2410 as well, since it might relate to his territory ^^
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 believe in the previous set up, the user should have written FirstName followed by the last name, so ideally the outcome should be similar. I tried to check on the DB but i can't see it directly...
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.
Sorry for the super delayed response - I hope you had an awesome Christmas! :PP
Totally; I just wonder whether we want to split those names from the previous setup (stored as displayName
) and run a process over our database, which splits those names in the same way, but then stores the result as firstName
, lastName
into the DB, so all users are stored the same way and we don't have to have an if/else when fetching the names.
That way we don't have to worry about it again, for example if we want to use the name in other areas.
AI-Generated Summary: This pull request consists of two patches. The first patch has changes across multiple files, while the second one just modifies the The first patch makes significant updates to the user profile page where the following enhancements are observed:
The second patch removes an unnecessary debug |
AI-Generated Summary: This pull request contains three patches. The first patch introduces the ability to display the user's first name, last name, and phone number on the profile page. It adds the ability to crop the profile picture and also updates the localization files to use "First Name" and "Last Name" instead of "Name". This patch equips the 'antd-img-crop' library, modifies 'MyAccount.tsx' to handle first name and last name separately, and adjusts their respective translations in English, French, and German. The second and third patches are minor amendments to 'MyAccount.tsx' that remove two console logs. These logs used to output "First Name" and "Last Name" when a user updated their name on the profile page. These changes clean up the code, removing log outputs that are no longer needed. |
AI-Generated Summary: This pull request involves changes in the user profile management section of a web application. The patch includes updates to the package.json and package-lock.json, adding a new library 'antd-img-crop' for image cropping. Moreover, changes are also made to 'MyAccount.tsx' to include the ability to display and modify the user's first and last name separately, rather than displaying a single name. The handling of the user's profile picture is adjusted to incorporate image cropping, and console logs are removed to clean up the code. Translation files ('translation.json') have been updated accordingly in English, French, and German to account for the separation of first and last names. In patches 2 to 4, console log statements have been removed for tidying up. |
user can change first last name in my account page
Phone number is only displayed for now
Add cropping to profile picture
Known limitations:
If user created account with 2 first names, the display will be wrong