Skip to content
This repository has been archived by the owner on Aug 7, 2024. It is now read-only.

Account Button replaced with User's GitHub Profile Picture #9703

Merged
merged 7 commits into from
Nov 17, 2023

Conversation

anirudhp26
Copy link
Contributor

@anirudhp26 anirudhp26 commented Oct 31, 2023

Fixes Issue

fixes #9691

Changes proposed

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • My change requires changes to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

Screenshots

Before
image
Before: Mobile Version
image

After
image
After: Mobile Version
image

Note to reviewers

Copy link
Member

@sital002 sital002 left a comment

Choose a reason for hiding this comment

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

Tested on:
Environment: Local Development
OS: Ubuntu
Browser: Firefox
works well but the image is too big and while hovering the borders are making it weird as well.
After changes:
image
image

@anirudhp26
Copy link
Contributor Author

Tested on: Environment: Local Development OS: Ubuntu Browser: Firefox works well but the image is too big and while hovering the borders are making it weird as well. After changes: image image

Then we can remove the borders or we can make them rounded, circle around the image.

I'll try different designs and let you know 👍

@anirudhp26
Copy link
Contributor Author

Check this out, changed the size to 40, border is now rounded, and also the alt text is more relatable

image

@Abhishek-90
Copy link
Contributor

Check this out, changed the size to 40, border is now rounded, and also the alt text is more relatable

image

This does look better than the previous hover effect.

@SaraJaoude SaraJaoude added the issue linked Pull Request has issue linked label Nov 2, 2023
@eddiejaoude
Copy link
Member

Please update the PR description with the latest screenshots

@anirudhp26
Copy link
Contributor Author

Done

@eddiejaoude
Copy link
Member

eddiejaoude commented Nov 2, 2023

Thank you 👍 please include the mobile responsive before/after also

@badrivlog
Copy link
Contributor

I'm not sure why the manage profile navigation link size is bigger than other links?
IMG_20231102_235024

It's my suggestion, I think for consistent design we should use same size for all navigation links, ensuring they look visually related and don't create confusion for users.

@anirudhp26
Copy link
Contributor Author

Thank you 👍 please include the mobile responsive before/after also

Surely, I'll include that

@anirudhp26
Copy link
Contributor Author

I'm not sure why the manage profile navigation link size is bigger than other links? IMG_20231102_235024

It's my suggestion, I think for consistent design we should use same size for all navigation links, ensuring they look visually related and don't create confusion for users.

but shrinking user's photo to that size won't look good according to me

Copy link
Member

@eddiejaoude eddiejaoude 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, looks good 👍

On mobile navbar does not close after clicking on profile picture like other nav links do.

Please also see inline comment.

@anirudhp26
Copy link
Contributor Author

Working on the navbar closing issue

Copy link
Member

@eddiejaoude eddiejaoude left a comment

Choose a reason for hiding this comment

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

Looks good, thank you 👍

@eddiejaoude eddiejaoude merged commit 2d15ac3 into EddieHubCommunity:main Nov 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue linked Pull Request has issue linked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Using Users GitHub's profile photo instead of Account Button in Navbar
7 participants