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

Hmt 3 navbar #2

Merged
merged 11 commits into from
Mar 7, 2024
Merged

Hmt 3 navbar #2

merged 11 commits into from
Mar 7, 2024

Conversation

justin-phxm
Copy link
Contributor

image

Added the navBar component

image

use these variables to test the conditional rendering.

const isSignedIn = true;
const hasTeam = true;

Copy link
Contributor

@ideen1 ideen1 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, just a few small changes

src/components/Header.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

This image has a little background left in it. If it's difficult to perfectly clean this one up, I think it is okay to use an icon from an library

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking that if we used a social sign on library or google auth, we could import their profile picture. If we used an icon, I am not sure if that would be possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

That’s a great idea. If we use an icon we can conditionally swap it out for an image tag. However, we can also keep the image approach you have. I think the image would just need to be exported from figma a little cleaner

public/CTCYYCLogo_Cropped 1.svg Outdated Show resolved Hide resolved
@ideen1 ideen1 self-requested a review February 23, 2024 06:26
@ideen1
Copy link
Contributor

ideen1 commented Feb 29, 2024

This looks good, just the svgs need to be cropped closer to the image so there's no border.

Copy link
Contributor

@ideen1 ideen1 left a comment

Choose a reason for hiding this comment

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

Profile SVG should have no border around it, otherwise LGTM!!

@justin-phxm justin-phxm self-assigned this Mar 1, 2024
@justin-phxm justin-phxm requested a review from ideen1 March 1, 2024 01:55
@justin-phxm
Copy link
Contributor Author

Profile SVG should have no border around it, otherwise LGTM!!

updated!
image

@ideen1
Copy link
Contributor

ideen1 commented Mar 5, 2024

Looks good! It can be merged in

@justin-phxm justin-phxm merged commit 3aeab6b into main Mar 7, 2024
3 checks passed
@justin-phxm justin-phxm deleted the HMT-3-navbar branch March 7, 2024 16:22
nataliey16 added a commit that referenced this pull request Mar 12, 2024
nataliey16 added a commit that referenced this pull request Mar 25, 2024
nataliey16 added a commit that referenced this pull request Apr 7, 2024
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