Skip to content
This repository has been archived by the owner on Feb 28, 2018. It is now read-only.

Display: inline-block on container not needed #29

Open
shankie-codes opened this issue Nov 26, 2015 · 6 comments
Open

Display: inline-block on container not needed #29

shankie-codes opened this issue Nov 26, 2015 · 6 comments

Comments

@shankie-codes
Copy link

Hi guys,

Great component! One slight issue: Is there any reason that you need to inline style display : inline-block on the container? I want my passwords to fill the width of their container, so this is an issue for me.

https://github.com/seethroughtrees/react-ux-password-field/blob/master/src/index.js#L257

Any reason that it can't be removed?

Thanks,

Andrew

@seethroughdev
Copy link
Owner

Thanks @shankiesan . Sorry for the delay.

You're right. That really doesn't need to be there. I can remove, but would need to bump to a breaking version, since I don't know if it could affect someone's layout.

I'm going to see if anything else needs to be done that could be breaking, and put it in the same release. Hard to know because I don't use this component anymore.

Going to leave this open. In the meantime, you okay with overriding the style?

@shankie-codes
Copy link
Author

Hey @seethroughtrees thanks for your reply. No stress, I've overridden it for now.

Makes sense to me to wait until a breaking version. Perhaps if you want to have some built-in styles, you could pass a styles object that contains all your default styles, but then if you wanted to override those styles, you could pass an empty styles={{}} object in props so that people could use their own style sheets.

@seethroughdev
Copy link
Owner

Yep, I think that would definitely be the way to do it. If you want to submit a PR, I'd be happy to merge it in. Otherwise, we can address when I have the time.

@shankie-codes
Copy link
Author

Cool, I'll see if I get time over Christmas when some client work settles down.

@seethroughdev
Copy link
Owner

np... thanks!

@LeBenLeBen
Copy link

+1

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

No branches or pull requests

3 participants