feat(JumpLinks): consumed Penta updates #10027
Conversation
| import { JumpLinksList } from './JumpLinksList'; | ||
|
|
||
| export interface JumpLinksItemProps extends Omit<React.HTMLProps<HTMLLIElement>, 'onClick'> { | ||
| /** Whether this item is active. Parent JumpLinks component sets this when passed a `scrollableSelector`. */ | ||
| isActive?: boolean; | ||
| /** Href for this link */ | ||
| href?: string; | ||
| href: string; |
There was a problem hiding this comment.
This needs to be required, as without it the anchor internal to the component cannot receive keyboard focus.
If the intent is that a consumer can pass either an href to link to an ID on the page, or the onClick to handle this functionality themselves, then we would need to render either an anchor with an href or a button with the onClick, not an anchor with either/both.
There was a problem hiding this comment.
Should we make a codemod to at least warn consumers about this?
There was a problem hiding this comment.
| <span className={styles.jumpLinksLinkText}>{children}</span> | ||
| </a> | ||
| <span className={styles.jumpLinksLink}> | ||
| <a className={css(buttonStyles.button, buttonStyles.modifiers.link)} href={href} onClick={onClick}> |
There was a problem hiding this comment.
Passed the classes in instead of using Button with props just to retain the onClick event type. No strong opinion about this, so I can just update the onClick event type and use the Button component proper here instead if we want.
There was a problem hiding this comment.
I would update this to use the Button if possible.
|
Preview: https://patternfly-react-pr-10027.surge.sh A11y report: https://patternfly-react-pr-10027-a11y.surge.sh |
thatblindgeye
requested review from
a team,
nicolethoen,
mfrances17,
andrew-ronaldson,
lboehling and
srambach
and removed request for
a team
| <span className={styles.jumpLinksLinkText}>{children}</span> | ||
| </a> | ||
| <span className={styles.jumpLinksLink}> | ||
| <a className={css(buttonStyles.button, buttonStyles.modifiers.link)} href={href} onClick={onClick}> |
There was a problem hiding this comment.
I would update this to use the Button if possible.
thatblindgeye
force-pushed
the
jumpLinksPenta
branch
from
0e5b35b
to
4a94da0
Compare
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #9994
Additional issues: