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

allow for styling the inner loader element #3

Closed
wants to merge 3 commits into from
Closed

allow for styling the inner loader element #3

wants to merge 3 commits into from

Conversation

danalloway
Copy link

this PR illustrates who we could possibly tweak all the Loader classes to allow for a style prop to be passed through and applied last to support overriding / customizing the inner loader element.

this will allow overrides and customizations during implementation
@danalloway danalloway mentioned this pull request Aug 6, 2017
@davidhu2000
Copy link
Owner

@danalloway this is a great idea. Are you interested in implementing this to every loader?

PS: PropTypes.object is forbidden. use PropTypes.shape()

@danalloway
Copy link
Author

danalloway commented Aug 9, 2017

@davidhu2000 do you have a source on PropTypes.object being forbidden? was unaware.

@danalloway
Copy link
Author

never mind I found it, it's an eslint rule regarding vague prop-types
https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/forbid-prop-types.md

will swap it out

@davidhu2000
Copy link
Owner

@danalloway are you still working on this? Let me know if you aren't, and I will implement this across all the loaders.

@danalloway
Copy link
Author

I am not

@davidhu2000
Copy link
Owner

davidhu2000 commented Aug 22, 2017

ok, in that case, when I get a chance, I will merge your PR and finish up this feature.

This PR fixes #2

Thanks for your contribution.

@RenanCostaNascimento
Copy link

That would be great! Do you have a ETA on this?

@davidhu2000
Copy link
Owner

@RenanCostaNascimento, top priority right now is server side rendering. Hopefully, I can test that out this weekend and merge the changes.

Once that's done, I will get to working on this.

@gwenf
Copy link

gwenf commented Dec 1, 2017

@davidhu2000 Hey, are you still working on this? I would like this functionality. Let me know if you are too busy and I will give it a shot.

@davidhu2000
Copy link
Owner

@gwenf, sorry for the late reply, I haven't had too much time to spend on this project. You are more than welcome to take a shot about this feature. Thank you.

@Dman757
Copy link

Dman757 commented Apr 4, 2018

@davidhu2000 Just checking if this is still actively being worked on, noticed a version release yesterday.

@jacargentina
Copy link

@davidhu2000 Yeah, any change to get this merged to master, and release a new version? I need to vertical align my loaders too!!

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.

None yet

6 participants