Skip to content
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

Added profile picture preview on successful upload #96

Merged

Conversation

sachintom999
Copy link
Contributor

Background

  • In the user profile page, when a user uploads his profile picture, there is no preview of the uploaded picture. The user is not sure whether the upload is success or not. The uploaded profile picture is getting saved on submit though.

Demo

before-fix

Solution

  • Add an image preview element in the html ( which is not displayed by default ) , but will be visible with the last uploaded image when the image upload is success

Note :

  • As per this solution, the Remove Button is hidden when user uploads a new picture, to avoid confusion. He can upload a new image to change the first uploaded image.
  • The button is shown until he changes the original profile picture.
  • Another observation is that, currently in the update profile page, we don't have a way to revert any unsaved text changes, unless he refreshes the page. Something like a Cancel button is a potential solution.
  • The functionality of the Remove Button which deletes the original picture is retained.

Changes

  • talent/templates/talent/profile_picture.html

Testing after fix

after-fix

@@ -1,8 +1,11 @@
<div id="profile-picture" class="flex items-center shrink-0 flex-col lg:w-[244px]">
<div id="profile-picture"
<div id="profile-picture-placeholder"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed id as it is same as the id of the div in line 1

Copy link
Collaborator

@dogukanteber dogukanteber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution @sachintom999. Normally, we avoid using Javascript as much as possible. Instead, we use Hyperscript. However, in this case, using Hyperscript instead of Javascript introduces more complexity, in my opinion.

I just wanted to give you an idea of how we do things.

After you modify the PR according to the comments I just left, we can merge it.

Once again, thank you for your work!

@sachintom999
Copy link
Contributor Author

Thank you for your contribution @sachintom999. Normally, we avoid using Javascript as much as possible. Instead, we use Hyperscript. However, in this case, using Hyperscript instead of Javascript introduces more complexity, in my opinion.

I just wanted to give you an idea of how we do things.

After you modify the PR according to the comments I just left, we can merge it.

Once again, thank you for your work!

hi @dogukanteber ,
Thank you for reviewing.
Noted your point of using Hyperscript. Will keep in mind in the future PRs.

I have made the requested changes and pushed.

@dogukanteber
Copy link
Collaborator

Well done, thanks!

@dogukanteber dogukanteber merged commit c823d51 into OpenUnited:main Oct 20, 2023
@sachintom999 sachintom999 deleted the fix-profile-picture-upload-preview branch October 20, 2023 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants