cs/move aria label

[devceline]

Issue: #5953

Effects

• Aria-label now forwarded to the modal instead of its child

Verify

• Edit the example modal to have an aria property like aria-label="foo". Previously the prop would be passed to the first child instead of the parent

Quick note:

This is my first time contributing to a big library like this, please let me know if anything should be changed in PR/code formatting. Thank you.

[kyletsang]

I think we should just forward aria-describedby and aria-label to the dialog. Don't need to map all aria properties.

[devceline]

I think we should just forward aria-describedby and aria-label to the dialog. Don't need to map all aria properties.

But isn't it more intuitive that the props you pass to a component go to what it is rendering directly, and also more accessibility friendly?

const Component = (props) => {
  return (
    <div {...props}> {/*It makes sense that props would be forwarded here*/}
      <span {...props}> </span> {/* instead of here */}
    </div>
  )

May I ask why single out only 2 aria props? Is there an accessibility bonus to giving to the child vs. the parent?

[kyletsang]

According to the accessibility docs on dialogs, you would only really need the two aria properties anyways (aria-label might be debatable because a modal should have a visible title?).

[devceline]

According to the accessibility docs on dialogs, you would only really need the two aria properties anyways (aria-label might be debatable because a modal should have a visible title?).

Oh alright, do you recommend I close this PR or should I just update it to accept aria-label optionally?

[kyletsang]

Feel free to amend this PR

[devceline]

Feel free to amend this PR

Amended the PR, hope this is satisfactory

[kyletsang]

Looks good. Could you add a test please?

[devceline]

Looks good. Could you add a test please?

Alrighty, added one :) Tried to follow the same test for the labelledby

[kyletsang]

Looks good, thanks!