-
-
Notifications
You must be signed in to change notification settings - Fork 157
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
UI related stories #93
Conversation
search bar & logo sticky"
- Minimizes distractions between the card vs. the map - Removes the need to make certain parts of the card sticky, since everything important is in view all the time - More fleshed out styling for stories - Typographic improvements for all page elements, including a rudimentary modular scale for headings and paragraphs - Addition of search form affordance (not yet wired up to anything) - Ability to hide and show the card to allow the user to concentrate on the map when desired. - There may be more, but these are the big tickets.
- Moves the intro content out of the sidecard and into its own component. Allows the sidebar card more space for functionality.
- Enables built-in Mapbox nav, which doesn't have an opinion about "up/down" vs. "north/south"
constructor(props){ | ||
super(props); | ||
this.state = { isPopped: true } | ||
this.handleIntroPopup = this.handleIntroPopup.bind(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you feel about using the fat arrow function syntax to get around having to bind every function to this
? I'm not sure if there's really a difference between the two ways, but it could save you some typing in the future :)
So in the function declaration, instead of
handleIntroPopup () {
// stuff
}
you can do
const handleIntroPopup = () => {
// stuff
}
and you'll have access to this
context, so you won't need to bind it in the constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely, @mirandawang! Good catch and I learned something :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating! :) So you should be able to delete all the .bind()
references now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done and done :)
- Uses fat arrow syntax - Fixes class vs. className error
- Removes uneeded call to `bind` in constructor - Removes uneeded `proptypes` import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lookin Good!! Thanks for your hard work! Feel free to merge this once you remove the extra bind statement from IntroPopup :) !
This work (hopefully) closes or obviates several stories related to the map card/sidebar.
I realize this is a big PR that touches a lot of things, and I don't necessarily require approval now since this is my "ui branch". Just wanted everyone to be aware so I can gather reactions, and keep pushing other items into this branch.