-
Notifications
You must be signed in to change notification settings - Fork 663
data-component adr part 5 #7867
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
base: main
Are you sure you want to change the base?
Changes from all commits
fff3f9d
dc1cc17
d886f3c
55b406e
c6b08e9
3cd151f
f3aa549
55e192e
24026be
bc3dc32
fd4e024
06c2480
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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@primer/react": minor | ||
| --- | ||
|
|
||
| Add data-component attributes and associated tests for ActionMenu, AnchoredOverlay, Autocomplete, and NavList |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -398,7 +398,10 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr | |
| data-side={cssAnchorPositioning ? side : position?.anchorSide} | ||
| > | ||
| {showXIcon ? ( | ||
| <div className={classes.ResponsiveCloseButtonContainer}> | ||
| <div | ||
| className={classes.ResponsiveCloseButtonContainer} | ||
| data-component="AnchoredOverlay.CloseButtonContainer" | ||
|
Member
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. I'd rename to data-component="AnchoredOverlay.CloseButton" and put it on the IconButton instead |
||
| > | ||
| <IconButton | ||
| {...(closeButtonProps as IconButtonProps)} | ||
| type="button" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,6 +55,20 @@ describe('Autocomplete', () => { | |
| <Autocomplete.Input {...props} /> | ||
| </Autocomplete> | ||
| )) | ||
|
|
||
| it('tags overlay with data-component when menu is shown', async () => { | ||
| const user = userEvent.setup() | ||
| const {container} = render( | ||
| <LabelledAutocomplete | ||
| menuProps={{items: mockItems, selectedItemIds: [], ['aria-labelledby']: 'autocompleteLabel'}} | ||
| />, | ||
| ) | ||
| const inputNode = container.querySelector('#autocompleteInput')! | ||
| await user.type(inputNode, 'z') | ||
|
|
||
| expect(container.querySelector('[data-component="Autocomplete.Overlay"]')).toBeInTheDocument() | ||
| }) | ||
|
Comment on lines
+69
to
+70
Member
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. missing: Autocomplete.Input, Autocomplete.Menu |
||
|
|
||
| it('calls onChange', async () => { | ||
| const user = userEvent.setup() | ||
| const onChangeMock = vi.fn() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,27 @@ const NextJSLikeLink = React.forwardRef<HTMLAnchorElement, NextJSLinkProps>( | |
| describe('NavList', () => { | ||
| implementsClassName(NavList) | ||
|
|
||
| it('renders data-component attributes for NavList and NavList.SubNav', () => { | ||
|
Member
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. missing: data-component tests for NavItems |
||
| const {container} = render( | ||
| <NavList> | ||
| <NavList.Item href="#">Item 1</NavList.Item> | ||
| <NavList.Item> | ||
| Item 2 | ||
| <NavList.SubNav> | ||
| <NavList.Item href="#">Sub Item 1</NavList.Item> | ||
| </NavList.SubNav> | ||
| </NavList.Item> | ||
| </NavList>, | ||
| ) | ||
|
|
||
| const nav = container.querySelector('nav') | ||
| expect(nav).toBeInTheDocument() | ||
| expect(nav).toHaveAttribute('data-component', 'NavList') | ||
|
|
||
| const subNav = container.querySelector('[data-component="NavList.SubNav"]') | ||
| expect(subNav).toBeInTheDocument() | ||
|
llastflowers marked this conversation as resolved.
|
||
| }) | ||
|
|
||
| it('supports TrailingAction', async () => { | ||
| const {getByRole} = render( | ||
| <NavList> | ||
|
|
||
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.
I kind of feel like this should be on the AnchoredOverlay instead (override)