Skip to content

Conversation

@kmcfaul
Copy link
Contributor

@kmcfaul kmcfaul commented Feb 9, 2023

Tooltips were not appearing for Th and Td because of the trigger wrapping div.

@kmcfaul
Copy link
Contributor Author

kmcfaul commented Feb 9, 2023

The DropdownToggleCheckbox change is unrelated to the bug, it is just breaking the build atm due to a removed prop still being present.

@patternfly-build
Copy link
Contributor

patternfly-build commented Feb 9, 2023

@kmcfaul

This comment was marked as outdated.

@kmcfaul

This comment was marked as outdated.

@kmcfaul kmcfaul linked an issue Feb 13, 2023 that may be closed by this pull request
@tlabaj tlabaj self-requested a review February 15, 2023 21:31
@kmcfaul
Copy link
Contributor Author

kmcfaul commented Feb 20, 2023

Traced the source of the extra tooltip: the SortColumn wraps the children with TableText, which is generating a separate tooltip if the text is truncated. This tooltip isn't exposed to the user, customizable, or removable. So when the Th tooltip prop is used, it is properly setting the tooltip for the th element, but the TableText element still renders its own tooltip.

This might be a larger discussion and require a followup refactor to determine how and where custom tooltips should be implemented for Table.

For this PR, I have updated Popper to allow both trigger and reference to be passed in together. If both are passed in, the wrapping div will be removed (because of reference) and the trigger will still be rendered by Popper instead of shifting the render to the component that is calling Popper. This is general change to make updates to other Popper consumers simpler for removing the wrapping div if it's causing issues, and still fixes the spacing issue, so can be merged after approvals now.

@thatblindgeye
Copy link
Contributor

thatblindgeye commented Feb 27, 2023

So it looks like the default tooltip for truncation is appearing fine on hover. Would we want to open a follow up regarding discussion/refactoring for custom tooltips being passed in since there still is a bug when passing one in?

I'm also noticing that tooltips don't appear when a truncated header receives focus. That might tie into a possible followup discussion/refactor, but since this PR is trying to resolve tooltips not appearing correctly it would be worth trying to include it here if it's a straight forward enough fix. Forgot that there's an issue regarding this already

@gitdallas
Copy link
Contributor

closed due to being covered in #8733

@gitdallas gitdallas closed this Mar 3, 2023
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

Successfully merging this pull request may close these issues.

Table: tooltips not appearing correctly

5 participants