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

Start content higher on landing page #16

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Start content higher on landing page #16

wants to merge 2 commits into from

Conversation

andyjiang3
Copy link

Fix #12

Copy link
Member

@audrey-yang audrey-yang left a comment

Choose a reason for hiding this comment

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

Header bar position looks good! Could you change two things?

  1. Keep the height of the landing image to be automatic (it's ok if it's a little cut off)
  2. Make sure there is no gap between the landing background and bar on smaller screens (same issue as For smaller screens, make sure there is no gap between the landing background and nav #4)

@andyjiang3
Copy link
Author

andyjiang3 commented Mar 19, 2022

Header bar position looks good! Could you change two things?

  1. Keep the height of the landing image to be automatic (it's ok if it's a little cut off)
  2. Make sure there is no gap between the landing background and bar on smaller screens (same issue as For smaller screens, make sure there is no gap between the landing background and nav #4)

I made the following changes. However, when you resize the window, the position of the header doesnt get updated right away, you need to reload the page for the position to update. I tried using useEffect to update it dynamically but the way the image load in and how the ref is set makes it complicated to get the height of the image. Lmk if theres a better way of doing this.

@audrey-yang
Copy link
Member

@andyjiang3 Hmm, I think bringing back Joe's change from #4 that used calc and adding min in the css file might work. Let me know if that still doesn't, though.

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.

Start content higher on landing page
2 participants