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

Adding profile photo #122

Open
wants to merge 6 commits into
base: gh-pages
Choose a base branch
from
Open

Conversation

simran054
Copy link

No description provided.

@ndattani
Copy link
Member

Hi @simran054, can you explain why a 1.59MB picture was added, then removed, then added again?

@AbdulMuhaymin can you check the dimensions and decide whether or not to approve the pull request? I think the space between the bottom of the chin and the bottom of the photo may be a bit too big, and the recommendation was always to follow the examples of my picture and Greg's, without over-doing it like in Diya's picture (a bit gets cut off when the picture goes on the website, it's best to look at the photos of myself and Greg, in the "images" folder, rather than looking at the photos on the website).

@simran054
Copy link
Author

Hi @ndattani sorry about that. Initially, I didn't add the image to the images folder, and when I tried to move it, I wasn't able to, so instead, I removed and re-added to the correct folder.

If the request can be rejected, I can make some more changes to the cropping and upload the image once more.

Copy link
Member

@AbdulMuhaymin AbdulMuhaymin left a comment

Choose a reason for hiding this comment

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

@simran054 I reviewed your commit. As suggested by @ndattani , please crop the image to remove the space between the bottom of your chin and the bottom of the image, and also remove some of the lateral empty space too.

Secondly, this is quite a high resolution image. The dimension that we were using is 270 x 285. Your image has a dimension of 1686 x 1779. That means you have the aspect ratio correct but not the exact value. I am not completely sure but I guess you should also reduce the dimension to 270 x 285 to match with others. So, removing some of the lateral space will also help you maintaining the aspect ratio after you remove the empty space below your chin.

In any case, do not delete the local repository, close this pull request or start a new pull request, please. After you made the suggested changes (cropping maintaining aspect ratio and centering, and then resizing to the desired size), you can commit and then push those changes. The change will be reflected in this pull request.

@ndattani
Copy link
Member

Hi @simran054, in the future if you don't know how to do something (such as moving a file from one folder to another folder), you an ask it in our "Git" channel in the HPQC Discord server (https://discord.gg/4B59r7ukhV).

The command git mv would be one option to use, for what you wanted to do. Also, how did you end up with an image with an aspect ratio of 1686 x 1779?

@simran054
Copy link
Author

simran054 commented May 29, 2024

@AbdulMuhaymin Thank you for the feedback; I made those adjustments. Please let me know if any further changes are required.
@ndattani Thank you. I will ask questions there moving forward.

Thank you both for your patience :)

@AbdulMuhaymin
Copy link
Member

@simran054 Thank you. The image SimranBhalla_fixed.png has correct dimension and I think it satisfies all the criteria except one, i.e., the naming of the file. It's very easy to fix.

You do not need to keep the high resolution image. You can delete that image (or move to someplace else in your computer other than this repository) and rename this newly uploaded fixed version as SimranBhalla.png. In this way, there will be only 1 file change from your side in this PR.

And since you need to change the image anyway, you may consider reducing the space a little bit more between the bottom of your chin and the bottom of the picture (maintaining aspect ratio). The ideal image in this case would be to follow Nike Dattani's image on https://hpqc.org/people.html. Notice that the space is almost zero there!

I can approve this PR ASAP after you commit these changes.

@simran054
Copy link
Author

@AbdulMuhaymin Please let me know if these changes are okay now. Thanks

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.

3 participants