Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add no_title support (and stories) for RelativeTime #2872

Merged
merged 9 commits into from
Jun 4, 2024

Conversation

smockle
Copy link
Member

@smockle smockle commented May 30, 2024

What are you trying to accomplish?

This is the Primer View Components version of primer/react#4635.

List the issues that this change affects.

Closes https://github.com/github/accessibility-audits/issues/5524

Risk Assessment

  • Low risk the change is small, highly observable, and easily rolled back.

This API was deliberately chosen to avoid breaking any existing RelativeTime usage that might be relying on the title attribute.

What approach did you choose and why?

First, this PR updates the version of the @github/relative-time-element custom element from v4.4.0 to v4.4.1, which includes no-title (via github/relative-time-element#283 and github/relative-time-element#284).

Second, this PR adds no_title support to the Primer ViewComponents RelativeTime component.

Third, this PR adds Lookbook stories to demonstrate the effects of no_title:

Accessibility

  • No new axe scan violation - This change does not introduce any new axe scan violations.

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.

Copy link

changeset-bot bot commented May 30, 2024

🦋 Changeset detected

Latest commit: be53b7d

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

Copy link
Contributor

⚠️ Visual differences found

Our visual comparison tests found UI differences. Please review the differences by viewing the files changed tab to ensure that the changes were intentional.

Review visual differences

@smockle smockle marked this pull request as ready for review May 31, 2024 18:33
@smockle smockle requested review from a team as code owners May 31, 2024 18:33
@smockle smockle requested a review from langermank May 31, 2024 18:33
@smockle smockle marked this pull request as draft May 31, 2024 18:33
@smockle
Copy link
Member Author

smockle commented May 31, 2024

Keeping this in draft until the version of github/relative-time-element is bumped.

Update: Opened #2882 to pull in the new version!

Copy link
Contributor

@joelhawksley joelhawksley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor semantics around boolean usage ❤️

app/components/primer/beta/relative_time.rb Outdated Show resolved Hide resolved
app/components/primer/beta/relative_time.rb Outdated Show resolved Hide resolved
…4.1 (#2882)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: primer[bot] <119360173+primer[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Lindsey Wild <[email protected]>
Co-authored-by: lindseywild <[email protected]>
Co-authored-by: Jon Rohan <[email protected]>
Co-authored-by: primer-css <[email protected]>
@smockle smockle marked this pull request as ready for review June 3, 2024 18:57
@smockle smockle requested a review from joelhawksley June 3, 2024 18:57
previews/primer/beta/relative_time_preview.rb Outdated Show resolved Hide resolved
previews/primer/beta/relative_time_preview.rb Outdated Show resolved Hide resolved
@smockle
Copy link
Member Author

smockle commented Jun 3, 2024

Release / npm / Canary (push) failed; sounds like a temporary issue:

error npm error code E409
🦋  error npm error 409 Conflict - PUT https://registry.npmjs.org/@primer%2fview-components - Failed to save packument. A common cause is if you try to publish a new package before the previous package has been fully processed.

I’ll re-run the failed check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants