Skip to content

Conversation

@siddharthkp
Copy link
Member

@siddharthkp siddharthkp commented Nov 4, 2025

@siddharthkp siddharthkp self-assigned this Nov 4, 2025
Copilot AI review requested due to automatic review settings November 4, 2025 13:29
@siddharthkp siddharthkp requested review from a team as code owners November 4, 2025 13:29
@siddharthkp siddharthkp requested a review from mperrotti November 4, 2025 13:29
@changeset-bot
Copy link

changeset-bot bot commented Nov 4, 2025

⚠️ No Changeset found

Latest commit: 1225188

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

@github-actions github-actions bot added the staff Author is a staff member label Nov 4, 2025
@siddharthkp siddharthkp enabled auto-merge November 4, 2025 13:30
@github-actions github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Nov 4, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2025

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request re-introduces the deprecated underline prop to the Link component for backwards compatibility. The changeset indicates this was originally planned as a breaking change (removing the prop), but the PR reverses that decision.

Key Changes

  • Added the deprecated underline prop back to the Link component with a deprecation notice
  • Updated CSS to support the data-underline attribute while maintaining inline link behavior
  • Added Storybook stories and visual regression tests for the underline feature
  • Removed the changeset that documented the breaking change removal

Reviewed Changes

Copilot reviewed 8 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/react/src/Link/Link.tsx Added back underline prop with deprecation warning and data-underline attribute
packages/react/src/Link/Link.module.css Added CSS rule to style links when data-underline='true' with backwards compatibility comment
packages/react/src/Link/Link.stories.tsx Added underline: false to playground args
packages/react/src/Link/Link.features.stories.tsx Added Underline feature story demonstrating the prop
packages/react/src/Link/Link.docs.json Added reference to the new Underline story in documentation
packages/react/src/Link/Link.dev.stories.tsx Expanded dev stories to test various combinations of inline and underline props
e2e/components/Link.test.ts Added visual regression test for the Underline story
.playwright/snapshots/.../Link-Dev-Inline-light-tritanopia-linux.png Updated visual regression snapshot reflecting new test combinations
.changeset/swift-queens-smoke.md Removed changeset file that documented the breaking change
Comments suppressed due to low confidence (2)

packages/react/src/Link/Link.module.css:31

  • The comment on line 27 states 'setting underline={false} does not override this' for inline links, but this behavior should be more clearly documented. The current implementation means inline links will always be underlined when the a11y setting is true, regardless of the underline prop value. This could be confusing for developers. Consider expanding the deprecation notice in the TypeScript prop definition to include this important caveat.
  /* Deprecated: but need to support backwards compatibility */
  &:where([data-underline='true']),
  /*
    Inline links (inside a text block), however, should have underline based on accessibility setting set in data-attribute
    Note: setting underline={false} does not override this
  */
  [data-a11y-link-underlines='true'] &:where([data-inline='true']) {
    text-decoration: underline;
  }

.changeset/swift-queens-smoke.md:1

  • The entire changeset file is being deleted, which suggests this PR is reversing a planned breaking change. However, since the underline prop is being re-introduced as deprecated, a new changeset should be created documenting this change as a patch or minor version bump (depending on your versioning strategy) that explains the prop is being maintained for backwards compatibility. This ensures proper release notes and version tracking.

@siddharthkp siddharthkp added this pull request to the merge queue Nov 4, 2025
Merged via the queue into main with commit e5029d7 Nov 4, 2025
54 of 57 checks passed
@siddharthkp siddharthkp deleted the revert-7021-liuliu/remove-link-underline-prop branch November 4, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm staff Author is a staff member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants