-
Notifications
You must be signed in to change notification settings - Fork 526
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
Update types for IconButton, ActionMenu.Button, TabNav.Link and internal consumers #2677
Changes from 1 commit
f8396de
22d54d0
ff81b15
821d888
4acd672
e731b2b
73b7829
4c5f3ee
e9d50df
ae1b022
96a6224
bf1f55e
b8fc520
a2a252e
a324bea
98f7989
6db1ab6
4761a20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import {ForwardRefComponent as PolymorphicForwardRefComponent} from './utils/polymorphic' | ||
import classnames from 'classnames' | ||
import {To} from 'history' | ||
import React, {useRef, useState} from 'react' | ||
|
@@ -67,18 +68,18 @@ function TabNav({children, 'aria-label': ariaLabel, ...rest}: TabNavProps) { | |
) | ||
} | ||
|
||
type StyledTabNavLinkProps = { | ||
export type TabNavLinkProps = React.DetailedHTMLProps<React.HTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement> & { | ||
to?: To | ||
selected?: boolean | ||
} & SxProp | ||
|
||
const TabNavLink = styled.a.attrs<StyledTabNavLinkProps>(props => ({ | ||
const TabNavLink = styled.a.attrs<TabNavLinkProps>(props => ({ | ||
activeClassName: typeof props.to === 'string' ? 'selected' : '', | ||
className: classnames(ITEM_CLASS, props.selected && SELECTED_CLASS, props.className), | ||
role: 'tab', | ||
'aria-selected': !!props.selected, | ||
tabIndex: -1, | ||
}))<StyledTabNavLinkProps>` | ||
}))<TabNavLinkProps>` | ||
padding: 8px 12px; | ||
font-size: ${get('fontSizes.1')}; | ||
line-height: 20px; | ||
|
@@ -105,9 +106,8 @@ const TabNavLink = styled.a.attrs<StyledTabNavLinkProps>(props => ({ | |
} | ||
|
||
${sx}; | ||
` | ||
` as PolymorphicForwardRefComponent<'a', TabNavLinkProps> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is used polymorphically, but the styled-components as types conflict some with the polymorphic helper types - overriding the styled-components version gives better typing, and avoids those issues |
||
|
||
TabNavLink.displayName = 'TabNav.Link' | ||
|
||
export type TabNavLinkProps = ComponentProps<typeof TabNavLink> | ||
export default Object.assign(TabNav, {Link: TabNavLink}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import React from 'react' | ||
import TabNav from '../TabNav' | ||
import {Button} from '../Button' | ||
|
||
export function shouldAcceptCallWithNoProps() { | ||
return ( | ||
|
@@ -20,3 +21,11 @@ export function shouldNotAcceptSystemProps() { | |
</> | ||
) | ||
} | ||
|
||
export function shouldAcceptButtonAsProps() { | ||
return <TabNav.Link as={Button} /> | ||
} | ||
|
||
export function shouldAcceptTabNavLinkprops() { | ||
return <TabNav.Link to="to something" selected as={Button} /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. validates that we can accept specific props, and as |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
broadens these to they work with arbitrary components, and not only primer octicons