feat(PPDSC-2403): Extend role and aria-haspopup on Popover

[jannuk59]

PPDSC-2403

• Extended the role and aria-haspopup on the popover component.
  Example:

<Popover content="..." role="menu">
  <IconButton aria-haspopup="true" />
</Popover>

• By default the role and aria-haspopup is to 'dialog'

I have done:

• Written unit tests against changes
• Written functional tests against the component and/or NewsKit site
• Updated relevant documentation

I have tested manually:

• The feature's functionality is working as expected on Chrome, Firefox, Safari and Edge
• The screen reader reads and flows through the elements as expected.
• There are no new errors in the browser console coming from this PR.
• When visual test is not added, it renders correctly on different browsers and mobile viewports (Safari, Firefox, small mobile viewport, tablet)
• The Playground feature is working as expected

Before:

After:

Who should review this PR:

How to test:

[newskit-engineering]

You can preview these changes on:
Documentation Site: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-2403-extend-role-and-aria-haspopup-on-popover
Storybook: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-2403-extend-role-and-aria-haspopup-on-popover/storybook

[mutebg]

I think the description of the ticket might be misleading.
you don't need to change this, people can pass a role on the popover and this will be overridden.

For example, I just did that with a passing banner:

[mutebg]

You don't have to remove that, this should stay as the default behaviour.

but we need to check if people provide they aria-haspopup and if so we don't apply it automatically.

For example:

<Popover content="..." role="menu">
  <IconButton aria-haspopup="true" />
</Popover>

That logic should happen in base-floating-element.tsx file, at the point you are cloning the children you can check if the children have already aria-popup

[mstuartf]

I don't think we need to pass this down and then back up again. The purpose of BuildAriaAttributesFn is to expose data from the BaseFloatingElement that the Popover can't access. But the Popover has access to children.props.

Can we simply move buildContextAriaAttributes inside the component and change it to:

const buildContextAriaAttributes: BuildAriaAttributesFn = ({
  floating: {id, open}
}) => ({
  'aria-haspopup': children.props['aria-haspopup'] || 'dialog',
  'aria-controls': open ? id : undefined,
});

I think this would achieve the same result without any changes to the base component.

What do you think?