react-ux-password-field icon indicating copy to clipboard operation
react-ux-password-field copied to clipboard

Display: inline-block on container not needed

Open shankie-codes opened this issue 9 years ago • 6 comments

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

shankie-codes avatar Nov 26 '15 17:11 shankie-codes

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?

seethroughdev avatar Nov 30 '15 18:11 seethroughdev

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.

shankie-codes avatar Dec 01 '15 09:12 shankie-codes

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.

seethroughdev avatar Dec 03 '15 18:12 seethroughdev

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

shankie-codes avatar Dec 03 '15 18:12 shankie-codes

np... thanks!

seethroughdev avatar Dec 03 '15 19:12 seethroughdev

+1

LeBenLeBen avatar Jan 15 '16 13:01 LeBenLeBen