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

Fixes #80, #106. Make the hamburger work with pure CSS. #111

Merged
merged 3 commits into from
Apr 7, 2017

Conversation

alxn
Copy link
Contributor

@alxn alxn commented Feb 26, 2017

I've been reading up on this, while there seems to be a movement against hamburgers, there's plenty of examples of pure-CSS ones: 1, 2. - I think these are all using a <input> for the clickable.

This PR adds that support, and I've put up a gh-pages site at https://alxn.github.io/minima/, it would be great if everyone could test that site and let me know in the comments how it works for them, which OS/Browser they were using, and I can update the below:

So far tested:

  • OS-X
    • Chrome
    • Safari
  • iOS
    • Chrome
    • Safari
  • Android

@alxn alxn changed the title Fixes #80, #93, #106. Make the hamburger work with pure CSS. Fixes #80, #106. Make the hamburger work with pure CSS. Feb 26, 2017
float: right;
width: 36px;
height: 36px;
z-index: 2;

Choose a reason for hiding this comment

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

perhaps also add cursor: pointer; here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ambrosiani I'd wondered about that, but it'll only make sense if the hamburger gets used for desktop. cursor: pointer; doesn't seem to make much sense for me if it's in "touch" ?

Choose a reason for hiding this comment

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

The hamburger menu appears in thin desktop windows as well. But perhaps it shouldn't be put here – I don't know how the SCSS media queries work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ambrosiani are you're right, I'll add it in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ambrosiani you should see it updated on my gh-pages site as well.

Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

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

👍 Tested successfully on Chrome on Androïd Marshmallow on a Moto g⁴.

@palermog
Copy link

Works for me on iOS 10.2.1 on an iPhone 6

@gatezh
Copy link

gatezh commented Apr 1, 2017

Is it going to be merged?

@alxn
Copy link
Contributor Author

alxn commented Apr 7, 2017

@gatezh that's up to the maintainers.

@pathawks
Copy link
Member

pathawks commented Apr 7, 2017

/cc: @jekyll/minima

@ashmaroli
Copy link
Member

Minima hasn't seen a merge in months now.. Mantainers, some ❤️ please.

@pathawks
Copy link
Member

pathawks commented Apr 7, 2017

@ashmaroli Aren't you a maintainer? Who's in charge here?

@ashmaroli
Copy link
Member

Aren't you a maintainer? Who's in charge here?

No, I don't have write-permissions here. I'm guessing @parkr and @benbalter is in charge of this repo

@benbalter
Copy link
Contributor

Is there consensus here that this should get merged? If so, please submit a code review and I can get this merged.

Also, if anyone is interested in becoming a maintainer of this project, please feel free to jump in and triage the existing open PRs (what can be merged, what should be closed, etc.) and we can go from there.

Copy link
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@benbalter
Copy link
Contributor

Wow @alxn! This is great. Thanks for the full request and for contributing to Jekyll!

@jekyllbot: merge +minor.

@jekyllbot jekyllbot merged commit cdbde06 into jekyll:master Apr 7, 2017
jekyllbot added a commit that referenced this pull request Apr 7, 2017
@gatezh gatezh mentioned this pull request Apr 10, 2017
domingohui pushed a commit to domingohui/minima that referenced this pull request Apr 14, 2017
domingohui pushed a commit to domingohui/minima that referenced this pull request Apr 14, 2017
pixyzehn added a commit to pixyzehn/pixyzehn.com that referenced this pull request Sep 16, 2017
@kurkweb
Copy link

kurkweb commented Jul 26, 2018

Guys, I must not be in the right head space or something. Am i just supposed to move the z-index of my toggle up? Here is my portfolio kurkweb.github.io . Side note, safari doesn't seem to like bootstrap css or jquery. I have seen other websites for people I am looking to hire me have slider issues on safari and hamburger issues as well. Pshh

hookex pushed a commit to hookex/red-star that referenced this pull request May 13, 2019
hookex pushed a commit to hookex/red-star that referenced this pull request May 13, 2019
@jekyll jekyll locked and limited conversation to collaborators Jul 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants