This repository has been archived by the owner on May 24, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 165
[terra-dropdown]Added aria-haspoup and aria-controls attribute #3936
Merged
Merged
Changes from 24 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
caadb3e
Added aria-properties
536f8c8
addde changelog
3044177
changed id to fix wdio issues
4bab927
snap update
0f71abe
id updated
fcb7910
id updated
08b4966
id updated
20141c9
id updated
62206f0
id updated
5ed5690
updated snap files
b766425
snap update
9a8b867
snap update
a81aa81
snap update
1172c73
snap update
fa63590
snap update
9e8e1bf
snap update
21ad9ba
snap update
1bd9327
snap update
468dbd3
Review Comments Implemented
0a11b55
Review Comments Implemented
3344405
Merge branch 'main' of https://github.com/cerner/terra-core into drop…
e68b94f
Review Comments Implemented
ea5ae0d
Review Comments Implemented
f8779db
Merge branch 'main' of https://github.com/cerner/terra-core into drop…
7cb2fd3
updated as per review comments
3768685
updated as per review comments
d5e7bb7
updated as per review comments
0d7aa0d
added custom Props.classname for dropdown button
fb23b80
Merge branch 'main' of https://github.com/cerner/terra-core into drop…
d029e14
Merge branch 'dropdown-aria-property' of https://github.com/cerner/te…
9c44d50
Merge branch 'main' of https://github.com/cerner/terra-core into drop…
5ac876d
Merge branch 'main' of https://github.com/cerner/terra-core into drop…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,10 +3,21 @@ import ThemeContextProvider from 'terra-theme-context/lib/ThemeContextProvider'; | |
import { IntlProvider } from 'react-intl'; | ||
/* eslint-disable-next-line import/no-extraneous-dependencies */ | ||
import { shallowWithIntl, mountWithIntl } from 'terra-enzyme-intl'; | ||
// eslint-disable-next-line import/no-extraneous-dependencies | ||
import { v4 as uuidv4 } from 'uuid'; | ||
import translationsFile from '../../translations/en.json'; | ||
/* eslint-disable-next-line import/no-extraneous-dependencies */ | ||
import DropdownButton, { Item, Variants } from '../../src/DropdownButton'; | ||
|
||
let mockSpyUuid; | ||
beforeAll(() => { | ||
mockSpyUuid = jest.spyOn(uuidv4, 'v4').mockReturnValue('00000000-0000-0000-0000-000000000000'); | ||
}); | ||
|
||
afterAll(() => { | ||
mockSpyUuid.mockRestore(); | ||
Comment on lines
+13
to
+18
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. Need to add similar spy for splitbutton test as well 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. Done. |
||
}); | ||
|
||
describe('Dropdown Button', () => { | ||
it('should render a default dropdown type', () => { | ||
const wrapper = shallowWithIntl( | ||
|
@@ -87,11 +98,13 @@ describe('Dropdown Button', () => { | |
it('correctly applies the theme context className', () => { | ||
const wrapper = mountWithIntl( | ||
<ThemeContextProvider theme={{ className: 'orion-fusion-theme' }}> | ||
<DropdownButton label="Primary Option"> | ||
<DropdownButton label="Primary Option" id="dropDown"> | ||
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. can we have jest test to ensure button is assigned with value passed by consumer as ID instead of the auto generated ID. 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. Done. |
||
<Item label="1st Option" onSelect={() => {}} /> | ||
</DropdownButton> | ||
</ThemeContextProvider>, | ||
); | ||
const dropDownButtonId = wrapper.find('#dropDown'); | ||
expect(dropDownButtonId.exists()).toBe(true); | ||
expect(wrapper).toMatchSnapshot(); | ||
}); | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
why do we need aria-LabelledBy for
ul
menu ?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.
As suggested by UX team and provided the reference URL to add aria-labelledby for dropdown menu
https://www.w3.org/WAI/ARIA/apg/patterns/menu-button/examples/menu-button-links/
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 think this will result in announcing text of button on navigating of each item which is not expected. So I would double check on using this attribute on
ul
menu.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 had a look on the button navigation and it is not announcing text of button on menu change, it is reading as 'menu label (1 of 4) menu item' please validate at your end as well if required.
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.
Screen.Recording.2023-10-20.at.12.10.45.PM.mov
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.
you can notice the reported behavior on the screen recording that VO did announced button text as
menu Export, Deafult dropdown
which is not correct for the first dropdown itemThere 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.
Removed aria-labelledby property as discussed.