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

Sticky header #59

Merged
merged 10 commits into from
Dec 17, 2022
Merged

Sticky header #59

merged 10 commits into from
Dec 17, 2022

Conversation

oluoluoxenfree
Copy link
Contributor

Header now sticky and button moved to the top; not sure if it should further to the right?

Show me

Screenshot 2022-12-08 at 18 33 47

REMEMBER: Attach this PR to the Trello card

@netlify
Copy link

netlify bot commented Dec 8, 2022

Deploy Preview for anchor-polyfill ready!

Name Link
🔨 Latest commit e55d88f
🔍 Latest deploy log https://app.netlify.com/sites/anchor-polyfill/deploys/639c8eadfe2daf00086501c5
😎 Deploy Preview https://deploy-preview-59--anchor-polyfill.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@oddbird oddbird deleted a comment from netlify bot Dec 8, 2022
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
public/demo.css Outdated Show resolved Hide resolved
@dvdherron
Copy link
Contributor

I want a sticky header for the anchor positioning demo page

Copy link
Contributor

@dvdherron dvdherron left a comment

Choose a reason for hiding this comment

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

The placement of the button looks good to me. Curious to hear if others prefer it inline on the opposite side.

I think the only thing remaining is putting the z-index on the header instead of nav?

public/demo.css Outdated Show resolved Hide resolved
* main: (48 commits)
  try new approach for testing dumb browsers and their dumb subpixel rounding
  adjust regex for tests
  adjust regex for tests
  Use rounded values for e2e tests
  chore(deps): Automated dependency upgrades
  empty commit to trigger deploy preview
  lint
  Support implicit anchors via anchor attribute.
  Back to official WPT repo
  Exclude irrelevant WPTs
  tests
  Bump actions/setup-python from 3 to 4
  chore(deps): Automated dependency upgrades
  Fix custom props passed through other custom props.
  fix tests
  Do not overwrite DOM platform fns
  chore(deps): Automated dependency upgrades
  Test against our own WPT branch
  Set passing tests to 0 for unknown results
  lint
  ...
@jgerigmeyer
Copy link
Member

The placement of the button looks good to me. Curious to hear if others prefer it inline on the opposite side.

@oluoluoxenfree @dvdherron I think there's plenty of horizontal space, it would make sense to me if we place the button on the far right and only wrap below on narrow screens.

@jgerigmeyer
Copy link
Member

@oluoluoxenfree How close are we to wrapping up this PR?

@oluoluoxenfree
Copy link
Contributor Author

@jgerigmeyer I can make the z-index change now but the grid change I'm not sure about; if it's fine to have it below for now I can fix it in another PR

@jgerigmeyer
Copy link
Member

@oluoluoxenfree Looks good. It's not urgent, but @dvdherron could you provide some suggestions for moving the button to the right side?

* main:
  Use env vars to disable WPT result builds
@dvdherron
Copy link
Contributor

@oluoluoxenfree Looks good. It's not urgent, but @dvdherron could you provide some suggestions for moving the button to the right side?

Missed this before I signed off yesterday.

My first instinct would be to do a named grid-template with the h1 spanning across the entire top row and the the nav and button sharing the second row, place at their respective start and end positions. It will something similar to what we have on OddSite:

image

I can sketch up something really quick if that helps @oluoluoxenfree

@oluoluoxenfree
Copy link
Contributor Author

that would help, thanks so much @dvdherron

@dvdherron
Copy link
Contributor

The buttton sits on the opposite side of the header now:

image

We could also but everything in one row if that space in the middle looks too big on wide screens:

image

@dvdherron
Copy link
Contributor

dvdherron commented Dec 16, 2022

I think this is ready for a review @stacyk @jgerigmeyer @oluoluoxenfree

Copy link
Member

@jgerigmeyer jgerigmeyer left a comment

Choose a reason for hiding this comment

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

Looks great! 🚀

@jgerigmeyer jgerigmeyer merged commit 81e7143 into main Dec 17, 2022
@jgerigmeyer jgerigmeyer deleted the sticky-header branch December 17, 2022 02:28
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