GlLink: Set external link label only with the icon

Justin Ho Tuan Duongrequested to merge
justin_ho-gl-link-external-alt-only-with-icon into main

What does this MR do?

Since !4528 (merged), link.vue binds :data-alt="externalLinkLabel" on all three template branches. It does not check any condition. So every GlLink gets data-alt="(external link)". This includes internal links, external links without the icon, and links styled as buttons.

Only one CSS rule reads the attribute:

.gl-link-external::after { content: " ↗" / attr(data-alt); }

That rule applies only when the gl-link-external class is present. So the attribute is dead on every other link. This has two effects:

  1. The attribute is wrong on internal links. It says "(external link)" on a link that is not external.
  2. The attribute appears in element.dataset on every link. GitLab spreads { ...el.dataset } into component props and provide in about 20 places.

This MR makes externalLinkLabel return undefined unless shouldShowExternalIcon is true. That is the same condition that adds the gl-link-external class. So the attribute now lands where the CSS reads it, and nowhere else.

Why this matters now

The unconditional attribute broke the @gitlab/ui bump in gitlab-org/gitlab!253439 (closed). 14 Jest suites failed, with 17 failed tests. None of the affected links had the gl-link-external class, so every failure came from the dead attribute.

I patched the installed package in gitlab-org/gitlab with this change and re-ran those 14 suites. All 128 tests and 18 snapshots pass, with no changes to that repo. So this fix unblocks the bump and needs no snapshot updates there.

How to validate

  1. Run the Vue 2 tests: cd packages/gitlab-ui && NODE_ENV=test yarn jest src/components/base/link/link.spec.js
  2. Run the Vue 3 tests: cd packages/gitlab-ui && NODE_ENV=test VUE_VERSION=3 yarn jest src/components/base/link/link.spec.js
  3. Check that all 51 tests pass in both runs.
  4. Revert the change to externalLinkLabel and run the tests again. Check that 5 assertions fail.

Screenshots or screen recordings

No visual change. The rendered icon and its announced label are the same.

Integrations

Does this MR meet the acceptance criteria?

This checklist encourages the authors, reviewers, and maintainers of merge requests (MRs) to confirm changes were analyzed for conformity with the project's guidelines, security and accessibility.

Toggle the acceptance checklist

Conformity

  • The “What does this MR do?” section in the MR description is filled out, explaining the reasons for and scope of the proposed changes, per “Say why not just what”.
    • For example, if the MR is focused on usage guidelines, addressing accessibility challenges could be added in a separate MR.
  • Relevant label(s) are applied to the MR.
  • The MR is added to a milestone.
  • Added the ~"component:*" label(s) if applicable.
  • A changeset is added when this MR will do a patch, minor, or major update. More information can be found in doc/changesets.md.

Components

Documentation

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • Security reports checked/validated by a reviewer from the AppSec team

Accessibility

If this MR adds or modifies a component, take a few moments to review the following:

  • All actions and functionality can be done with a keyboard.
  • Links, buttons, and controls have a visible focus state.
  • All content is presented in text or with a text equivalent. For example, alt text for SVG, or aria-label for icons that have meaning or perform actions.
  • Changes in a component’s state are announced by a screen reader. For example, changing aria-expanded="false" to aria-expanded="true" when an accordion is expanded.
  • Color combinations have sufficient contrast.

🤖 AI-generated

Merge request reports