remove defaultProp from required property in components/modal

[dsifford]

closes #16062

Let me know if you have any further questions/comments.

[gziolo]

I'm wondering whether we should update docs to reflect that it is optional instead. It seems to be safer.

[dsifford]

That works too! Won't hurt my feelings to close this PR if that's the preferred route 😝

[dsifford]

Though, I'm wondering if that will result in a component that renders with close buttons that aren't usable (or, rather, that when clicked don't do anything). Is that an a11y issue?

[aduth]

Though, I'm wondering if that will result in a component that renders with close buttons that aren't usable (or, rather, that when clicked don't do anything). Is that an a11y issue?

It would, and it is. Nothing really guarantees that a developer implementing onRequestClose would handle this appropriately, but it is expected that a developer should be using the prop to manage the closing of the modal (and as such, its existence is at least appropriate as a signal that it's being handled).

Reflecting on this, I think it would be perfectly reasonable to mark this prop as "Required". Normally I would be concerned about the backwards-compatibility impact of removing this default prop, but since it is explicitly documented as required, I think it would be fair to let this go through as proposed.

[aduth]

Per comments, I think this is an appropriate improvement. I'd wait to merge pending @gziolo's thoughts.

[gziolo]

Per comments, I think this is an appropriate improvement.

Agreed, it will at least give some feedback when this prop isn't provided but the end effect will be the same. I didn't consider it.