patternfly-react icon indicating copy to clipboard operation
patternfly-react copied to clipboard
verified publisher icon patternfly

feat(Table): add tooltip for truncated Td, docs(Filter): add filter demos

Open kmcfaul opened this issue 3 years ago • 1 comments

What: Closes #7965

Adds remaining filter demos (attribute search, same select filter group, mixed select filter group, faceted filter).

Updates all filter demos to have the following:

  • Bulk select respects filtered state
  • Menu ids are unique between demos
  • Checkbox column space is maintained when there is an empty state
  • Tds are truncated to maintain column widths while filtering

While adding the last bullet I also noticed that no tooltip is added when Tds are truncated like Ths should I added that functionality as well.

Other updates in this PR:

  • Td: Added tooltip prop, and when using the truncate modifier, Td will attempt to make a default tooltip like Th
  • Th: Updated internal canDefault variable name to canMakeDefaultTooltip (matched in Td as well)
  • MenuToggle: Added missing ref spread to split button variant of the MenuToggle
  • Menu: added null check in createNavigableItems callback that can throw a console error if the menu closes due to the extraKeys handler

kmcfaul avatar Sep 20 '22 20:09 kmcfaul

Preview: https://patternfly-react-pr-8024.surge.sh

A11y report: https://patternfly-react-pr-8024-a11y.surge.sh

patternfly-build avatar Sep 20 '22 20:09 patternfly-build

@kmcfaul looks great overall!! Only noticed one little bug in the last demo, where there should be a badge showing the number of selections - and it seems to work for the first attribute, but not for the second. As soon as you select anything from the second attribute, the badge disappears.

https://user-images.githubusercontent.com/51165119/192321668-2a140037-1cd3-4492-a938-9b3e5e531539.mov

mmenestr avatar Sep 26 '22 15:09 mmenestr

Fixed the faceted filter badge. @mmenestr @wise-king-sullyman

Not sure why but the check being inline wasn't quite executing right (it was only looking at the first condition), probably a syntax thing. Moving the check above to a const lets it work properly.

kmcfaul avatar Sep 30 '22 14:09 kmcfaul

The tooltip prop is for specifying a custom tooltip when a tooltip is rendered (when there is the truncate modifier and the text length overflows). I don't think it forces the tooltip but I'm not sure. It should behave the same as the Th prop. @nicolethoen @mcoker

We may want to open a follow up for the extra tooltips as I'm not sure what's happening there at the moment. I added a tooltip to Td but I don't think that would be causing a Th to render a second tooltip.

kmcfaul avatar Oct 03 '22 17:10 kmcfaul

Your changes have been released in:

Thanks for your contribution! :tada:

patternfly-build avatar Oct 04 '22 14:10 patternfly-build