tooltip icon indicating copy to clipboard operation
tooltip copied to clipboard

Determining the availability of props.visible

Open devdammit opened this issue 8 years ago • 4 comments

Hello Guys! I noticed that you check the props.visible by the presence of it in props. Why not check for undefined?

https://github.com/react-component/tooltip/blob/b5e43fd3128f6bec414c7bb0308a4f998105118a/src/Tooltip.jsx#L72

We use a rule eslint - react/require-default-props. And set props.visible as undefined in the defaultProps. And if the props.visible is equal to undefined, then he still gets to extraProps.popupVisible.

devdammit avatar Aug 23 '17 13:08 devdammit

But why, are you going to lint code in node_modules.

benjycui avatar Aug 24 '17 01:08 benjycui

@benjycui, No, of course, we do not check node_modules into the style code. :) We have our UIkit, and we made a wrapper for rcTooltip. In this wrapper we describe props

devdammit avatar Aug 24 '17 08:08 devdammit

Why not check for undefined?

Yep, you can PR.

But I don't agree with this ESLint rule, we can set default prop for defaultXXX, but I don't think we should set default props for controlled props(e.g. value, visible..)

benjycui avatar Aug 25 '17 08:08 benjycui

@benjycui I agree with @mochachai, need to check it for undefined. For example in my case I need to use rc-tooltip in two different scenes inside the one component.

  1. Will work like a standard tooltip
  2. Will show preview with always opened tooltip

So I was going to just pass true or undefined in 'visible' props, but now I have to do one trick:

const props = {};
if (preview) {
  props.visible = true;
}

return (
  <Tooltip placement="bottom" {...props} />
)

oldwin avatar Nov 29 '17 16:11 oldwin