Fix #385

[FunctionDJ]

@kennethnym Does this library have any default text in any other components? What i mean is: Should it have a default text in english or only have this HTML attribute if it was set by the user to not assume anything like the language? Also: What should be the prop's name? Just aria-label?

[kennethnym]

I think it is a good idea to have a default label for the close button. As for the prop name, how about closeButtonAriaLabel? We could also accept a prop for props that should be forwarded to the button, like buttonProps:

<Modal.Card.Header
  showClose
  buttonProps={{
    'aria-label': 'Close this modal',
  }}
/>

cc @couds

[couds]

I was thinking that maybe will be better to add a guide on storybook or readme on how to use your own delete bottom. Because we will end up generating a lot of props like buttonProps on several components.

So by default we do not add this aria label, instead we explain how to add your own button. So the use it's free to add anything.

At the end of the day the showClose append the button, so can be easily be added outside the component.

It's not a perfect solution but I think it's better to start adding a ton of customs props for each case scenario that will add complexity to the library.

WDYT?

[kennethnym]

I definitely agree with you.

[FunctionDJ]

@couds I kind of disagree. I think the default components should at least support accessibility features. If they don't, i wouldn't have them in the first place, which would mean either the modal component has a prop like this that gets passed to the button element, or you always need to add the close button yourself.

I understand and agree with your point for the "slippery slope" where a decision to add this prop could result in a lot more props that could become managable. Therefore i would remove this button and force the user to add it themselve. Then again, this would be a breaking change for "no good reason". My position is simply that default features should be able to be fully accessible.

[couds]

@FunctionDJ the main issue with the default props/default it's de language, I don't want to assume the language. In this case I prefer to change the default behavior to not show the button showClose=false by default

[FunctionDJ]

@couds i agree with that reasoning. i'll add commits to the PR when i can. where would this breaking change of not showing the close button by default need to be documented?

[kennethnym]

It will be documented in the changelog when the next major version is released.