-
Notifications
You must be signed in to change notification settings - Fork 4
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
Rewrite Finder using React, Gatsby, and DS (WIP) #24
Conversation
Remind me to make small commits more often
src/pages/index.js
Outdated
import qs from "query-string"; | ||
|
||
class IndexPage extends Component { | ||
constructor(props) { |
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.
Not a big deal, but the pattern of a static for initial state + automatic function binding is preferable:
https://github.com/withspectrum/spectrum/blob/alpha/src/components/chatInput/index.js#L76-L123
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.
the majority of this page component should be relocated to its own component/broken up a bit. most of this functionality should be render-able from any page on the site.
Fix scripts
src/components/ClubCard.js
Outdated
@@ -131,7 +130,7 @@ class ClubCard extends Component { | |||
<Photo src={`/school/${data.id}.jpg`} ready={ready} /> | |||
</Box> | |||
<Flex p={3} justify="space-around" flexDirection="column" style={{ flex: 1 }}> | |||
<Heading.h4>{data.name}</Heading.h4> | |||
<Heading.h4 style={{ textTransform: "capitalize" }}>{data.name}</Heading.h4> |
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.
try the caps
prop :)
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.
I don't want it to be uppercased 😛
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.
oh! yes. every time I see textTransform
I figure it's uppercase. sorry, will quietly duck out of the room now :)
I need to make smaller commits more often
Apply fix from hackclub/design-system#5
No description provided.