Skip to content

Commit

Permalink
fix: Polishing the expandable container component (#2293)
Browse files Browse the repository at this point in the history
Fixes: #2048  

Asked to review and polish the expandable container component in labs. The Long Title example has an aria-label string interfering with the Target button's text. The Avatar example has a text alternative that isn't necessary.

[category:Components]

Release Note:
Update aria attributes on example for expandable container for better accessibility and remove alt text on avatar.
  • Loading branch information
williamjstanton authored Jul 25, 2023
1 parent efa6888 commit 1fe2317
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 19 deletions.
37 changes: 20 additions & 17 deletions cypress/integration/ExpandableContainer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ describe('ExpandableContainer', () => {
});

it('should have aria-expanded set to false', () => {
cy.findByRole('button', {name: 'expand container'}).should(
'have.attr',
'aria-expanded',
'false'
);
cy.findByRole('button')
.should(
'have.attr',
'aria-expanded',
'false'
);
});

it('should not show expanded content', () => {
Expand All @@ -28,42 +29,44 @@ describe('ExpandableContainer', () => {

context('when the container is clicked', () => {
beforeEach(() => {
cy.findByRole('button', {name: 'expand container'}).click();
cy.findByRole('button').click();
});

it('should set aria-expanded to true', () => {
cy.findByRole('button', {name: 'expand container'}).should(
'have.attr',
'aria-expanded',
'true'
);
cy.findByRole('button')
.should(
'have.attr',
'aria-expanded',
'true'
);
});

it('should show expanded content', () => {
cy.findByText('Content').should('be.visible');
});

it(`should have an aria-controls attribute pointing to the id of content`, () => {
cy.findByRole('button', {name: 'expand container'}).then(button => {
cy.findByRole('button').then(button => {
const buttonAriaControlsValue = button.attr('aria-controls');
cy.findByText('Content').should('have.attr', 'id', buttonAriaControlsValue);
});
});

context('when the container is clicked again', () => {
beforeEach(() => {
cy.findByRole('button', {name: 'expand container'}).click();
cy.findByRole('button').click();
});

it('should hide expanded content', () => {
cy.findByText('Content').should('not.be.visible');
});

it('should have aria-expanded set to false', () => {
cy.findByRole('button', {name: 'expand container'}).should(
'have.attr',
'aria-expanded',
'false'
cy.findByRole('button')
.should(
'have.attr',
'aria-expanded',
'false'
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion modules/labs-react/expandable/lib/ExpandableAvatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ const StyledAvatar = styled(Avatar)<StyledType>({
export const ExpandableAvatar = createComponent('button')({
displayName: 'Expandable.Avatar',
Component: ({altText, ...elemProps}: ExpandableAvatarProps, ref) => {
return <StyledAvatar altText={undefined} as="div" ref={ref} size={32} {...elemProps} />;
return <StyledAvatar altText="" as="div" ref={ref} size={32} {...elemProps} />;
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import testAvatar from './test-avatar.png';

export const LongTitle = () => (
<Expandable>
<Expandable.Target aria-label="expand container" headingLevel="h4">
<Expandable.Target headingLevel="h4">
<Expandable.Icon iconPosition="start" />
<Expandable.Avatar url={testAvatar} />
<Expandable.Title>
Expand Down

0 comments on commit 1fe2317

Please sign in to comment.