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 catch for intro members w/o profile in nav #345

Merged
merged 2 commits into from
Aug 25, 2022

Conversation

mroley1
Copy link
Contributor

@mroley1 mroley1 commented Aug 25, 2022

while logic was in place to handle intro members without accounts on profiles.csh.rit.edu in the body there is none for the navbar. This is a potential fix. catches errors and directs them to the null@csh.rit.edu default profile picture

current behavior for intro member:
nav

@jabbate19
Copy link
Collaborator

Hey Matt! Thank you for making a PR to help packet! I think you are on the right track for this, but I think a more robust and neat solution would be pulling the Gravatar of the specific user logged in. The profile picture will work fine for all upperclassmen since we have accounts, so you'd need to find a way to identify its not an upperclassmen (there should be a util somewhere in here) and then get from gravatar like how the images do for your packet itself. If you need more clarification, please lmk!

@mroley1
Copy link
Contributor Author

mroley1 commented Aug 25, 2022

Thank you for your feedback, I've never used HTML logic so I'm not sure if this would work but hopefully this is a better solution

@jabbate19
Copy link
Collaborator

I'll run a test later. You should totally come to floor later today! I don't remember if I've met you in person yet!

Copy link
Collaborator

@mxmeinhold mxmeinhold 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 to me, pending @jabbate19's review.

Copy link
Collaborator

@jabbate19 jabbate19 left a comment

Choose a reason for hiding this comment

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

Lgtm!

@galenguyer galenguyer merged commit d6d5ede into ComputerScienceHouse:develop Aug 25, 2022
@galenguyer galenguyer mentioned this pull request Aug 25, 2022
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.

4 participants