feat(Truncate): added tooltip props and anchor usage#11801
feat(Truncate): added tooltip props and anchor usage#11801nicolethoen merged 3 commits intopatternfly:mainfrom
Conversation
|
Preview: https://patternfly-react-pr-11801.surge.sh A11y report: https://patternfly-react-pr-11801-a11y.surge.sh |
ed88595 to
e774eac
Compare
| const minWidthCharacters: number = 12; | ||
|
|
||
| export interface TruncateProps extends React.HTMLProps<HTMLSpanElement> { | ||
| export interface TruncateProps extends Omit<React.HTMLProps<HTMLSpanElement | HTMLAnchorElement>, 'ref'> { |
There was a problem hiding this comment.
Could omitting ref be problematic if people were trying to use it in this component before? Do we need to make sure to forward a ref or something here?
kmcfaul
left a comment
There was a problem hiding this comment.
I like the refactor of refToGetParent to tooltipProps, its a little easier to understand I think
wise-king-sullyman
left a comment
There was a problem hiding this comment.
I share the concern about this possibly being a breaking change for passing ref, but other than that this looks good to me.
rebeccaalpert
left a comment
There was a problem hiding this comment.
Same as everyone else about the ref thing; otherwise it looks great to me. :)
e774eac to
0a65ff1
Compare
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
* feat(Truncate): added tooltip props and anchor usage * Added tests * Forwarded ref
What: Closes #10843
Some notes about this draft:
hrefprop) and a more customizable way to do it+add truncation to more complex things like buttons (viatooltipPropsprop and the triggerRef property) -- question is do we want to support truncating in things like buttons? You can do it now, it just doesn't apply the tooltip "correctly" (since the trigger is the text itself, the tooltip will render visually "inside" the button)Additional issues: