Tooltip - unable to control using *open* props

[tomkdgun]

Tooltip - unable to control using open props

What package(s) are you using?

Same happens also for version 6.x:

• [ ] carbon-components
• [ x] carbon-components-react

Detailed description

Unable to control visibility state of the Tooltip (open/close), using already existing open prop. It could be easy observed using provided codesandbox.

What offering/product do you work on? Any pressing ship or release dates we should be aware of? We need this functionality in IBM SPSS Statistics. Tooltips are the last components we are trying to migrate from ap-components-react to Carbon.

Steps to reproduce the issue

1. Assign Tooltip open prop to state variable (e.g. showTooltip).
2. Change value of state variable (true->false, false->true)
3. Result: Tooltip doesn't react on changes on open prop.

Expected: Tooltip will react on change for open prop

Please create a reduced test case in CodeSandbox https://codesandbox.io/embed/codesandbox-lo5ox

Issue exists on both carbon-components-react versions 6.x and 7.x. We are expecting a fix for version 6.x.

It's a blocking issue for us.

[tomkdgun]

@dakahn Any update on this issue ?

[dakahn]

None yet. Haven't had a chance to do any first aid on this issue just yet. @asudoh might have a workaround though

[asudoh]

A workaround may be force-remount technique.

[tomkdgun]

force-remount is not the good solution for us since tooltip wraps input fields.

[tomkdgun]

@asudoh @dakahn Do you know when this issue will be fixed ?\ We need to upgrade to Carbon 10 (IBM SPSS Statistics), but we are blocked by this issue, even to upgrade completely to Carbon 9.

[emyarod]

it looks like this is caused by the handleClickOutside method not accounting for whether or not the component is controlled. a quick and dirty fix is to force the tooltip to accept an outside click listener (<ClickListener onClickOutside={ this.props.handleClickOutside || this.handleClickOutside }>) via props, but we'll need to warn the user about passing in a click listener if the component is controlled

@tomkdgun here is an example of the controlled tooltip functioning properly with the show/hide buttons (although it does not account for clicking the tooltip itself yet) https://deploy-preview-3357--carbon-components-react.netlify.com/?path=/story/tooltip--test, the code is currently a draft in #3357

@asudoh I think we may need to allow refs to be passed into the tooltip trigger node as well as the floating tooltip body to properly determine the source of click events

once we have confirmed the direction we want to take with this fix I can backport this to v9 as well

[tomkdgun]

@emyarod I've looked into the example, but the issue is more general. In our code we are changing open prop programatically based inside custom validation methods, e.g. during typing in input text/number field. So user doesn't perform clicks, but it types values, we can even validate 3 input fields values during edit of one, and based on the result open tooltip with proper message. Buttons in sandbox was just used to create simple example. BTW, it seems there is no animation/transition during showing tooltip in draft example, not sure if this is intended.

[emyarod]

@tomkdgun I may be misunderstanding but the open prop can be controlled no? I updated the example to include a text input which will open the tooltip if the input has an empty value and close the tooltip if the input is populated as a simple test

[tomkdgun]

@emyarod When this issue was opened, open prop didn't work at all (controlled tooltip). We have found this on version 9.x. Later on we created simple example with buttons, which didn't work on both versions, 9.x and 10.x.

1. Could you try if example you created works for 9.x version ?
2. What if you remove this empty function handler handleClickOutside={() => {}} ?

[emyarod]

@tomkdgun that external handleClickOutside is needed only when click events are involved, like in your example of toggling open with buttons. without it, the text input validation example should still function

I can backport the fix to v9 after we confirm the issue is resolved, just want to understand the issue first so I'm demoing in v10 with the netlify builds

edit: the behavior appears to be identical in v9, as the component itself is unchanged

[asudoh]

The use case brought up (opening tooltip as user types something IIUC) makes me feel that definition tooltip or icon tooltip is more suitable for the use case. The usage scenario of the interactive tooltip is for letting user open the tooltip by user gesture.

[tomkdgun]

@emyarod I've updated https://codesandbox.io/embed/codesandbox-lo5ox, by adding the example with controlled tooltip around input. This is simplified example from our app, since open also dependent on value validation. Tooltip should be open when input is focused. Unfortunately tooltip still handles clicks, which bypass/changes state of tooltip, even open prop is true.

Current behaviour:

1. When input is focused/blur using Tab key (no clicking) it works as expected, tooltip is open when input field is focused.
2. When input is focused using click, it seems first tooltip is open because field is focused, and just after this click is handled, tooltip is closed. Additionally additional clicks on focused input field are changing tooltip state, is open or closed, same for clicking on spinners.

[tomkdgun]

Tooltip content(text) is empty in case of valid input, tooltip should be opened only in case of invalid value (controlled). Because of this issue (still handling clicks on controlled tooltip), user can open controlled tooltip, which in case of empty text results with sth like this: @emyarod @abbeyhrt @asudoh

[emyarod]

@tomkdgun the codesandbox example appears to be unchanged from the original example. were your changes saved?

[tomkdgun]

@emyarod Try this one: https://codesandbox.io/embed/codesandbox-jilyl

[emyarod]

@tomkdgun is the issue addressed in the latest Netlify preview for #3357?

[tomkdgun]

@emyarod It seems it works, but please check if uncontrolled tooltip works as expected. I've found that this.props.open is false by default (if not provided), so this.props.open !== null will be always true. Probably default for open could be changed to null.

[emyarod]

oh yeah maybe a check for undefined would be correct here instead of a null check

[astha-jain]

ok, This looks like a big discussion going on here. I am facing somewhat similar issue here. My observation is - passing open prop to Tooltip component, I am not able to control Tooltip open or close by changing the open prop. Here is the codesandbox link explaining the problem

https://codesandbox.io/s/codesandbox-uqirr?fontsize=14

The solution according to me would be by fixing this line of code here. https://github.com/carbon-design-system/carbon-components-react/blob/v6/src/components/Tooltip/Tooltip.js#L281

const { prevOpen } = state; prevOpen should get the value from state.open and not prevOpen from state. It should be const { prevOpen } = state.open;

Please see if this is the same problem or not and suggest me if I need to open different issue to fix this.

[emyarod]

@astha-jain you mean const prevOpen = state.open right? since prevOpen isn't a property of state.open we won't destructure it

@asudoh does prevOpen taking the value of state.open instead of state.prevOpen look good to you? this change is what would properly sync props and state for the handleMouse method I believe (see latest git diff)

[asudoh]

@emyarod Do you want to put the diff that you propose...? Thanks!

[emyarod]

@asudoh sure, that change has already been made in draft #3357. would we also need to change gDSFP for our other components in a separate PR?

will need to update tests for #3357 but first want to make sure the changes to gDSFP and tooltip click listener address @tomkdgun and @astha-jain's issues

[asudoh]

@emyarod Thanks - state.prevOpen is for keeping track of property-initiated change in open (filtering out other change to state.open), so the change seems to change the purpose of the original gDSFP logic. That said, wondering what made you thought of making such change.

[emyarod]

@asudoh right now keeping const { prevOpen } = state will cause props and state to become out of sync somewhere in the handleMouse method, but it seems that const { open: prevOpen } = state avoids that desync

[asudoh]

I thought controlling the open state by open prop is not working given it wasn't at some point, but I see it actually is working with latest 7.x as well as with latest 6.x. You can see it by adding a knob for open prop in the Storybook. Wrt the case of CodeSandbox link in the issue description, it appears that "close-on-click-outside" is happening as soon as the tooltip gets open by hitting the button - Which explains why it works with Storybook knob which is in a different frame, and it doesn't with the button in the demo. That said, a viable workaround is setting open state via setTimeout() - I can see it works by making such change to the CodeSandbox.

[tomkdgun]

@asudoh What in case when click on trigger is performed (input wrapped by tooltip) ? Click handler to open/close tooltip in this case should be skipped, when Tooltip is controlled (open prop provided).

[asudoh]

Skipping user interaction controlled case will be an interesting idea, will take a note.

[tomkdgun]

@asudoh @emyarod When fix will be ready and released ? It seems that some fix were prepared almost 2 weeks ago, which probably require some minor correction and tests on uncontrolled Tooltip, see https://github.com/carbon-design-system/carbon/issues/2849#issuecomment-514610555

[asudoh]

As I stated, you don't have to wait for our fix - setTimeout(stateUpdater, 0) will work for you.

[tomkdgun]

@asudoh Its difficult to apply this for 40, 200 or 400 places

[tomkdgun]

@asudoh @emyarod Could you remove "status: waiting for author's response" and assign someone to this issue ? Do fix will be available in next Carbon 10.5.x release ?

[emyarod]

@tomkdgun are there any issues that are left unresolved in the latest netlify preview for #3357?

[tomkdgun]

Latest netlify works well for my scenario (controlled one). Probably you need to just check if most common scenario (uncontrolled one) still works. This issue is very important for IBM SPSS Statistics.

Edit: Actually I've noticed that on netlify, under Tooltip component test, there are default one examples, which it seems doesn't work, probably because of things I've noticed under this comment https://github.com/carbon-design-system/carbon/issues/2849#issuecomment-514610555

[tomkdgun]

@asudoh I've tried workaround you have suggested, with setTimeout. Unfortunately it doesn't work as expected for wrapped input. e.g. I'm changing open with setTimeout to true, tooltip opens, but additional clicks inside tooltip trigger (input) are changing internal Tooltip state (closing/opening). My code around tooltip can't change internal state in such situation since parent component is not rendered/updated, open prop doesn't change e.g. always true, while internal Tooltip open state is changing on clicks.

[emyarod]

there are default one examples, which it seems doesn't work, probably because of things I've noticed under this comment #2849 (comment)

I don't quite understand what you wrote in your edit, can you elaborate on the issue? there should be an example of an uncontrolled tooltip and another example of a controlled tooltip right?

and the open prop in the default Tooltip story should also be uncontrolled

[tomkdgun]

@emyarod Yes, defaultTooltip story doesn't specify open, Tooltip is uncontrolled, which is correct. In latest netlify, test works fine, but defaultstory, doesn't.

[emyarod]

@tomkdgun default tooltip story appears to be working correctly for me (using the open storybook knob), and both tooltips in test story also appear to be working correctly. can you tell me more about the issue you are seeing?

[tomkdgun]

@emyarod Can we talk on Slack ? https://w3.ibm.com/bluepages/profile.html?uid=P40065820 On https://deploy-preview-3357--carbon-components-react.netlify.com/?path=/story/tooltip--default-bottom I can't close tooltip (previously working by click) On https://deploy-preview-3357--carbon-components-react.netlify.com/?path=/story/tooltip--no-icon and other with icon, I can't open tooltip (previously working by click)

[asudoh]

To shed some light on this topic; The reduced case in this issue description (https://codesandbox.io/embed/codesandbox-lo5ox) can be changed below way so the demo works:

--- test.js.orig    2019-08-21 12:38:12.000000000 +0900
+++ test.js 2019-08-21 12:38:01.000000000 +0900
@@ -15,13 +15,13 @@
       <div>
         <Button
           style={{ padding: "15px 20px", margin: "4px 20px" }}
-          onClick={() => this.setState({ value: false })}
+          onClick={() => { setTimeout(() => { this.setState({ value: false }) }, 0); }}
         >
           Hide
         </Button>
         <Button
           style={{ padding: "15px 20px", margin: "4px 20px" }}
-          onClick={() => this.setState({ value: true })}
+          onClick={() => { setTimeout(() => { this.setState({ value: true }) }, 0); }}
         >
           Show
         </Button>

[tomkdgun]

@asudoh On Buttons yes, but on input wrapped by Tooltip it doesn't, because clicks are perform on tooltip trigger, not outside, and there is not change in state and condition in parent component. This is latest codesandbox https://codesandbox.io/embed/codesandbox-jilyl Edit: Please try to click inside input, or on spinner buttons.

[asudoh]

<Tooltip triggerText={<input>} /> (seen in https://codesandbox.io/embed/codesandbox-jilyl) looks a completely different one from here that should be tracked as a separate issue. It requires UX feedback to see if it's a valid used case.

[tomkdgun]

@asudoh Jun 11: https://github.com/carbon-design-system/carbon/issues/2849#issuecomment-500691617 Jul 22: https://github.com/carbon-design-system/carbon/issues/2849#issuecomment-513719069

[emyarod]

reopening for v9

[emyarod]

since this fix actually requires a breaking change, it isn't suited for backporting to previous versions