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

AppBar no blue stuff #10

Merged
merged 4 commits into from
Feb 17, 2017
Merged

AppBar no blue stuff #10

merged 4 commits into from
Feb 17, 2017

Conversation

jtrein
Copy link
Contributor

@jtrein jtrein commented Feb 15, 2017

  • adds non-intrusive styles for a couple components
  • AppBar is not blue anymore, but a more official Sussol orange (stolen from the website buttons)

@jtrein jtrein requested a review from Chris-Petty February 15, 2017 02:15
@Chris-Petty
Copy link
Contributor

Might I suggest using the MUI theme provider to apply colours across the app. Gotta leverage those Material-UI features http://www.material-ui.com/#/customization/themes

@jtrein
Copy link
Contributor Author

jtrein commented Feb 16, 2017

I tried, but couldn't figure it out. It's supposed to use React Context internally (inside it's own MUI component) but I can't find evidence of it passing properties to the children.

Anyway, they're moving away from the inline styles in the future as it hurts render performance (e.g. re-drawing CSS each render)

For now, maybe it's best to leave them, or centralize them and import them into each component? It'd be a dreadful exercise to pass them down from the parent one-by-one.

Copy link
Contributor

@Chris-Petty Chris-Petty left a comment

Choose a reason for hiding this comment

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

Looks fine to me. Can keep this project pretty simple.

In general I assume things are tested working and just look at coding stuff.

@Chris-Petty
Copy link
Contributor

@jtrein

For now, maybe it's best to leave them, or centralize them and import them into each component? It'd be a dreadful exercise to pass them down from the parent one-by-one.

Yea that is what we do in mobile. We have a folder called globalStyles to break down all our styles into areas, but all kept in the same place. https://github.com/sussol/mobile/tree/master/src/globalStyles

@Chris-Petty Chris-Petty merged commit b2c1d0e into master Feb 17, 2017
@Chris-Petty Chris-Petty deleted the appbar-no-blue branch February 17, 2017 02:38
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