[terra-popup] Changed disabledHeader to true as default

cerner / terra-framework

Terra framework houses composed and higher order react components to help developers quickly build out new applications.

http://terra-ui.com

Apache License 2.0

67 stars 71 forks source link

[terra-popup] Changed disabledHeader to true as default #2171

Closed adavijit closed 5 months ago

adavijit commented 5 months ago

Summary

What was changed:

Enabled close button for popup
Modified tests for the close button

Why it was changed: Should be able to focus close button to close the popup

Testing

This change was tested using:

[x] WDIO
[x] Jest
[ ] Visual testing (please attach a screenshot or recording)
[ ] Other (please describe below)
[ ] No tests are needed

Reviews

In addition to engineering reviews, this PR needs:

[ ] UX review
[ ] Accessibility review
[ ] Functional review

Additional Details

This PR resolves:

UXPLATFORM-10398

Thank you for contributing to Terra. @cerner/terra

sugan2416 commented 5 months ago

@adavijit

Doc examples of popup are causing page break

popup content area has reduced, adding a vertical scroll. Can you check why this has changed and since when?

Since all test examples of popup have set isHeaderDisabled to false, shouldn't doc examples be updated as well?

sdadn commented 5 months ago

The targetRef prop was being set as required in #2158.

This is also a breaking change that also needs to be reverted.

saket2403 commented 5 months ago

The targetRef prop was being set as required in #2158.

This is also a breaking change that also needs to be reverted.

It was previously required prop I had made it optional in the previous release and reverted the change in 2158 as the change was not required and the prop being optional would have broken the popup if not used. As per latest discussion in JRB (see JIRA 10398) Closing this PR.