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

Theme integration #54

Merged
merged 31 commits into from
Nov 9, 2018
Merged

Theme integration #54

merged 31 commits into from
Nov 9, 2018

Conversation

hetmann
Copy link
Collaborator

@hetmann hetmann commented Nov 6, 2018

  • added withGalio, GalioProvider, GalioTheme

@hetmann hetmann added the Type: Enhancement New feature or request label Nov 6, 2018
@hetmann hetmann self-assigned this Nov 6, 2018
@paullaros
Copy link
Contributor

Neat! Really like the naming conventions you're using.

@palingheorghe
Copy link
Collaborator

Hi,
I encountered some problems while testing out this in a different app.

How I found out about the problems:

  1. I initialized another Expo app. (React v16.5, Expo v31.0.2)
  2. I added all the necessary files from the 'theme' branch ( I basically transferred the 'src' folder and renamed it in 'components' )
  3. in my App.js I imported GalioProvider and withGalio.
  4. I have now tried several different ways of playing around with these.

Using GalioProvider
From my understanding this component's behavior should be the following:

  • The children of GalioProvider should have acces to our default theme through Context
  • I should be able to pass down my own theme through the Provider

Right now I cannot find any way of configuring the default theme (or creating a completly different theme) and then pass down my new or modified theme through the Provider. (The Provider doesn't accept any new value as it's using a static variable pre-defined in the theme/index.js file)

Using withGalio
The behavior I was expecting:

  • The component I'm using with withGalio should be able to acces my theme through the styles variable
const styles = theme => StyleSheet.create({
  container: {
    flex: 1,
    backgroundColor: theme.COLORS.FACEBOOK
  }
});
export default withGalio(App, styles);

This is working ✅ . My bgColor turned blue using the variable like this in a View component.

<View style={this.props.styles.container}>

I think we need to better define the purpose of both functions and maybe create some sort of config function in order to configurate the already existent theme or create a new one.
The only way of creating a new theme, right now as far as I see, is to go directly into the theme folder and modify the existing file which is not the best UX for people who want to use Galio as their main UI library.

Thanks! 🤙🏽

@palingheorghe
Copy link
Collaborator

palingheorghe commented Nov 8, 2018

✅Both 'withGalio' and 'GalioProvider' functions work correctly now.

We'll just have to change the package.json file (more about this above) and we're good to go, at least from my perspective.

Thanks! ❤️🤙🏽

@hetmann
Copy link
Collaborator Author

hetmann commented Nov 8, 2018

✅Both 'withGalio' and 'GalioProvider' functions work correctly now.

We'll just have to change the package.json file (more about this above) and we're good to go, at least from my perspective.

Thanks! ❤️🤙🏽

Just updated to version 0.4.0 that fixed the custom theme pass from GalioProvider
To use it install the latest version npm install galio-framework@latest or yarn add galio-framework@latest

@AntelaBrais
Copy link
Contributor

Amazing work @hetmann 👌

@hetmann
Copy link
Collaborator Author

hetmann commented Nov 8, 2018

Just to inform you guys & girls :) I'm writing a small README for the theme to understand how to use it & customise your components. I'll publish the updates in no time :)

@palingheorghe
Copy link
Collaborator

This is it! We're now ready to merge this beautiful new feature into the Alpha-1 branch.

Good job everyone! Nice!

🤙🏽

@palingheorghe palingheorghe merged commit c969f43 into Alpha-1 Nov 9, 2018
@palingheorghe palingheorghe deleted the theme branch September 11, 2019 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants