Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DropdownItem tooltip not shown when isDisabled #6102

Closed
cben opened this issue Aug 1, 2021 · 4 comments
Closed

DropdownItem tooltip not shown when isDisabled #6102

cben opened this issue Aug 1, 2021 · 4 comments

Comments

@cben
Copy link
Contributor

cben commented Aug 1, 2021

Describe the issue. What is the expected and unexpected behavior?
Actions with tooltip prop only show it when the action is enabled. When both isDisabled and tooltip props are given, no tooltip shows.
(My use case is action objects returned from Table with actionResolver, but I see that's all passed to regular Dropdown containing DropdownItem and it also reproduces there, so let's focus on DropdownItem.)

Please provide the steps to reproduce. Feel free to link CodeSandbox or another tool.
https://codesandbox.io/s/crimson-snowflake-k2snz
The 2 enabled actions show tooltips on hover, the 2 disabled ones ignore the tooltip prop.
I'd expect them to show the tooltip in both cases.

Is this a bug or enhancement? If this issue is a bug, is this issue blocking you or is there a work-around?
Bug. I don't know sure if this worked before, but we had code setting tooltips for disabled actions for a long time — to explain to users why the action is not available — and this is preventing users from seeing the explanations.

What is your product and what release version are you targeting?
OpenShift Cluster Manager, code using this is already in production.

@cben
Copy link
Contributor Author

cben commented Aug 1, 2021

@elad661 found the cause:

disabled elements in PF have pointer-events: none which means they don't get the hover event that triggers the tooltip.

Indeed, disabling that CSS rule in the browser makes the tooltip appear for all actions.
But is it a safe fix? I updated codesandbox link ^^ to include href / onClick props and the 2 disabled actions are still effectively disabled, but I don't know what other effects it may have 🤷

@cben
Copy link
Contributor Author

cben commented Aug 1, 2021

Another approach could be to render a structure similar to <Tooltip ...><Button isDisabled ... /> </Tooltip> which does work — attaching the tooltip to an outer span, with the pf-m-disabled only on inner element.

EDIT: which is exactly what the code tries to do:

const renderWithTooltip = (childNode: React.ReactNode) =>
tooltip ? (
<Tooltip content={tooltip} {...tooltipProps}>
{childNode}
</Tooltip>
) : (
childNode
);
, but in the end it renders into a single element that's both the tooltip trigger and disabled? I don't understand enough how Tooltip & FindRefWrapper work to debug this...

@cben
Copy link
Contributor Author

cben commented Aug 2, 2021

Turns out in OCM we tend to use extra span or div between Tooltip and Button:

    const addUserBtn = (
      <Button onClick={() => { setTimeout(() => openModal('add-user'), 0); }} variant="secondary" className="access-control-add" isDisabled={disabled}>
        Add user
      </Button>
    );
...
          {disabled ? (
            <Tooltip content={tooltipContent}>
              <span>
                {addUserBtn}
              </span>
            </Tooltip>
          )
            : addUserBtn}

With such span, the tooltip shows, without it doesn't. 💡
Note how the span gets aria-describedby="pf-tooltip-..." while the inner button gets pf-m-disabled class:
Ekrankopio de 2021-08-02 15-48-27

Arguably, having to do that is silly and error-prone. A direct <Tooltip ...><Button isDisabled ... /> </Tooltip> structure SHOULD work.

So perhaps Tooltip itself should be considered buggy?
Instead of doing things like React.cloneElement(children, { 'aria-describedby': id }) to its child — and using that directly as the Popper trigger element, maybe it should always wrap the child in a span?
Or maybe Popper should be responsible for wrapping?

@cben
Copy link
Contributor Author

cben commented Aug 2, 2021

Oh, I should have searched before opening new issue.

@cben cben closed this as completed Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant