Add Dialog::Header close_scheme, close_label params

[myabc]

What are you trying to accomplish?

Currently it is not possible to specify a custom ARIA label for the Dialog close button. This is problematic for apps (like OpenProject) with fully localized UIs.

This PR adds two new params to Primer::Alpha::Dialog::Header#initialize:

• @param close_scheme [Symbol] Whether the component can be closed with an "x" button. Either :close or :none.
• @param close_label [String] The aria-label text of the close "x" button.

This allows for a few new scenarios:

• localization: the caller can pass a localized string as close_label: argument (e.g. with I18n.t).
• completely hiding the close button: this may be desirable from a UX perspective if the caller renders a close button elsewhere in the dialog (e.g. the footer).

Screenshots

custom close label:

without a header close button:

the initial focussed element is the footer close button.

Integration

This should not be a breaking change.

List the issues that this change affects.

Closes # (type the GitHub issue number after #)

Related OpenProject Work Package: https://community.openproject.org/wp/61631

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?

This API is modelled on the Primer::Alpha::Banner component, which accepts dismiss_scheme and dismiss_label arguments.

My initial approach was to create a close_button slot - I went ahead and pushed that work (see #3374) but wasn't happy with the API. For example, in order the hide the close button, you would need to call with_close_button without a block.

Anything you want to highlight for special attention from reviewers?

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)
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• 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]

⚠️ No Changeset found

Latest commit: e6cf575

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[myabc]

Leaving this in draft - @HDinger, could you have a look?

[HDinger]

As discussed, I'd vote for splitting this PR into two: One for changing the label which is pretty straightforward imho. The other one for making the close button optional which is probably more controversial.

[HDinger]

Schemes are usually something like :danger, :primary and so on.. Since you want to achieve a boolean flag to hide and show the close button, it maybe makes sense to make that a boolean like show_close_button.

[github-actions]

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.