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

Export defaultStyles #147

Closed
kyeotic opened this issue Apr 6, 2016 · 8 comments
Closed

Export defaultStyles #147

kyeotic opened this issue Apr 6, 2016 · 8 comments

Comments

@kyeotic
Copy link
Contributor

kyeotic commented Apr 6, 2016

This is a feature request. I would like to be able to globally alter the default styles for the modal without needing to wrap the component. Exporting the default styles, or exporting a function that modified the default styles, would facilitate this pretty easily.

@claydiffrient
Copy link
Contributor

@tyrsius In the next release #100 will be available allowing you to override the default styles by use of CSS classes. Is this adequate for your use case?

@kyeotic
Copy link
Contributor Author

kyeotic commented Apr 6, 2016

Its a nice step, but it doesn't fully solve this. I would still have to either provide those classes to every Modal instance, or wrap it with my own component to put the classes on.

Allowing the defaultStyles object to be modified would allow globally styling the modal without wrapping it.

These cases are definitely similar, but I think there is still value in exporting the default object.

@claydiffrient
Copy link
Contributor

Cool cool, I'll dig in and see what I can do. It likely won't be in the next release, but I can probably get it in shortly after that.

@kyeotic
Copy link
Contributor Author

kyeotic commented Apr 6, 2016

I can make a PR for this if you'd like. Since you are exporting ModalPortal as the default, moving the styles into their own module would probably make sense. Not sure where you want it, it doesn't really make sense in components

lib/styles.js
OR
lib/styles/default.js

Either of those look good?

@claydiffrient
Copy link
Contributor

A PR would be awesome. I think lib/styles.js would be okay. I don't think we want to support multiple themes so I don't think specifiying lib/styles/default is necessary.

@kyeotic
Copy link
Contributor Author

kyeotic commented Apr 6, 2016

I split it up, but I didn't consider that the built module doesn't surface everything, just the Modal component. I'm not sure what the best way to do that is. It feels weird to tack it on to the Modal...

@kyeotic
Copy link
Contributor Author

kyeotic commented Apr 6, 2016

#148 should take care of this

@claydiffrient
Copy link
Contributor

Closed because of #148 being merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants