jodhman / react-eyedrop

Seamlessly integrate a static typed, fully-tested color-picking React component/hook!
24 stars 11 forks source link

Can't change button text to empty #7

Closed gundamftw closed 4 years ago

gundamftw commented 4 years ago

I want the button to display without any text, but passing in nothing to the Eyedropper component will revert it back to the default 'eye-drop' text

<EyeDropper></EyeDropper>

jodhman commented 4 years ago

Hey @gundamftw! Glad to see you're still using this. I will implement an opt-out property for the defaultText.

gundamftw commented 4 years ago

I just removed the default text completely because there is really no point to have one anyway. People who are going to use the eyedropper functionality should all be using the custom component, and they're going to use their own text on their button anyway.

jodhman commented 4 years ago

Yes. I see your point. Albeit something I promise in the documentation is minimal setup for basic functionality. One could perhaps argue whether or not having a default text helps achieve that.

gundamftw commented 4 years ago

You can achieve that by telling the users to use their own text in the documentation.

jodhman commented 4 years ago

But that's my point. People very often skim through documentation, just to get a working module, before deep diving for wanted functionality. By minimizing distance for them to reach the deep-dive stage, we'll ultimately make things easier. Let's keep a default text, maybe something else than we had before, and update the documentation both tipping and showing how to change that text. In your empty text case, one could simply set the buttonText to an empty string.

gundamftw commented 4 years ago

I believed that people will know what to do when they see an empty button.

And to implement this, you'll need to declare another property for storing the user input, and then required the user to declare that property in order to enter their own button text . And all of these steps just to have a default text on the button that is going to be change immediately, this just doesn't make sense.

jodhman commented 4 years ago

Yeah, you're probably right. :stuck_out_tongue_winking_eye: We'll go with your changes from the PR.