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

Hamburger menu #9

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Hamburger menu #9

wants to merge 6 commits into from

Conversation

i-ron-y
Copy link
Contributor

@i-ron-y i-ron-y commented May 11, 2018

Hamburger menu is functional. No shelf view yet.

Close #8

Copy link
Member

@psiemens psiemens left a comment

Choose a reason for hiding this comment

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

Nice! I like the way you structured the React components. Apart from the change I commented on in the code, can you make it so the hamburger shows a pointer hand on hover?

src/App.js Outdated
constructor(props) {
super(props);
this.state = {
menuVisible: false,
Copy link
Member

Choose a reason for hiding this comment

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

Small thing, but I like to preface boolean states with is -- i.e. isMenuVisible

@i-ron-y
Copy link
Contributor Author

i-ron-y commented May 28, 2018

Thanks! Fixed and fixed - didn't think of having the hamburger show a pointer hand because I was thinking only in terms of touch-screen mobile!

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