Missing space between Dialog Header and Body

HDinger commented 2 months ago

What are you trying to accomplish?

Add top-padding for the Dialog body to move it closer to the React component and avoid it being glued to the divider.

Screenshots

	Before	After
Default
With large header and divider
Custom header
Header filter

List the issues that this change affects.

Closes #3008

Risk Assessment

[ ] Low risk the change is small, highly observable, and easily rolled back.
[ ] Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
[ ] High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

Looking at the React component, which always has a padding-top for the body of a dialog
In Rails this was explicitly canceled (probably because the divider is not always visible as in React??). However, this results in the body being glued to the header which looks super strange
I am aware that the spacings are sill not optimal and consistent with the React component, however I think that this might already be a good improvement and would fix the related issue.

Accessibility

Fixes axe scan violation - This change fixes an existing axe scan violation.
No new axe scan violation - This change does not introduce any new axe scan violations.
New axe violation - This change introduces a new axe scan violation. Please describe why the violation cannot be resolved below.

Merge checklist

[ ] Added/updated tests
[ ] Added/updated documentation
[ ] Added/updated previews (Lookbook)
[x] Tested in Chrome
[x] Tested in Firefox
[x] Tested in Safari
[x] Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: d3e7ec9152bb2d29bb39d240fce5f5ab20a0ca1f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

| Name | Type | | ----------------------- | ----- | | @primer/view-components | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions[bot] commented 6 days ago

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.

HDinger commented 6 days ago

Hi @team can I please get any feedback on this?

primer / view_components