Skip to content

fix: make icon marks decorative and announce links that open a new tab (#3212) - #3225

Open
frano-m wants to merge 3 commits into
mainfrom
fran/3212-portal-link-a11y
Open

fix: make icon marks decorative and announce links that open a new tab (#3212)#3225
frano-m wants to merge 3 commits into
mainfrom
fran/3212-portal-link-a11y

Conversation

@frano-m

@frano-m frano-m commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #3212.

Two accessibility defects around the portal marks, plus the same redundant-alt pattern in three more components.

Before, IconLink rendered the mark and the anchor as siblings, both labelled with the same string, and the anchor opened a new tab with no signal:

<img alt="CAP" height="20" src="…/cap.svg"/>
<a href="https://celltype.info/…" rel="noopener noreferrer" target="_blank">CAP</a>

so a screen reader announced "CAP, image" then "CAP, link", and nothing said the link leaves the site.

Changes

components/common/IconLink/iconLink.tsx

  • The mark is now decorative (alt="") — the adjacent link text already names the destination.
  • Links that open a new tab append a visually-hidden cue to their accessible name.

The cue lives in one shared component. NewTabCue renders the visually-hidden text and owns the decision, so IconLink delegates rather than computing it — leaving IconLink simpler than it was before this branch.

The cue is derived, not asserted. IconLink has two consumers with different link kinds, so an unconditional cue would have been wrong:

Consumer URL Target
AnalysisPortalCell https://celltype.info/… _blank
HCABiologicalNetwork /hca-bio-networks/breast same tab

Rather than have callers declare it, the component uses the same two predicates Link uses to choose its target:

const opensInNewTab = !isClientSideNavigation(url) && isValidUrl(url);

so the cue cannot drift from where the link actually opens.

Three more components with the same redundant altsectionBioNetworkAtlases, bioNetworkCell and heroTitle each rendered the network icon with alt set to the text sitting right beside it. None is a portal link (one is a click handler, one an internal link, one a heading), so only the decorative-alt half applies to them. These are beyond #3212's stated scope, included deliberately as the same defect.

Standalone images keep their alt text — verified across the export, the only images still carrying alt are the header and footer logos (63 pages each), the BICCN and metadata-schema graphics, and the trademark mosaic ball on /about/attributions. None sits beside text repeating it.

Verification

Audited every icon image and every anchor in the generated export:

network/portal <img> with alt=""   232
cues wrongly on same-tab links       0
cues missing on _blank links         0

Every anchor carrying the cue, by link text — all _blank, none same-tab:

'CZ CELLxGENE'      x167      'CAP'                x2
'HCA Explorer'        x6      'UCSC Cell Browser'  x1
7 source study citations (via #3196's titleAccess)

The two IconLink cases on the atlas overview page, side by side:

alt=''  text='CAP'             target=_blank    new-tab cue=YES
alt=''  text='Breast Network'  target=same tab  new-tab cue=no

Surfaces exercised: the source datasets table's Explore column (CAP), the atlas overview integrated-objects table (CAP, CZ CELLxGENE ×167, UCSC Cell Browser), the network link in the atlas overview side column, the home page network cards, the network table cells, and the atlas hero.

npm run lint (0 errors), npm run check-format, npx tsc --noEmit, npm run build-prod:data-portal (64/64) all pass.

The Primary Data link is included

#3212 listed the HCA Data Explorer link in the Primary Data column, and half of that is now stale: #3199 shipped it as text only, with no mark, so it never had the duplicate-alt problem.

It did still open in a new tab announcing nothing, because it renders a plain Link rather than going through IconLink. Sharing the cue as NewTabCue made fixing that a one-line change rather than a duplicated span, so it is in this PR — the six HCA Explorer links now carry the cue, verified above.

🤖 Generated with Claude Code

frano-m and others added 3 commits September 11, 2026 09:10
Iconlink rendered the portal mark with alt set to the same string as the
adjacent anchor, so the mark was announced as an image and again as the link
name. The mark is now decorative. Links that open a new tab append a
visually-hidden cue to their accessible name, derived from the same predicates
link uses to pick its target - so internal links, which stay in the same tab,
correctly get no cue.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sectionbionetworkatlases, bionetworkcell and heroTitle each rendered the
network icon with alt set to the same text sitting next to it, so screen
readers announced the network twice. Same defect as the portal marks in
iconlink; none of the three is a portal link, so only the decorative alt half
applies. Standalone images - the header and footer logos, the biccn and schema
graphics, the trademark ball - keep their alt text.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…3212)

Extracts newtabcue so the "opens in a new tab" cue and the predicates that
decide whether it applies live in one place, and uses it for the primary data
column's plain link as well as iconlink. That link goes to the hca data
explorer in a new tab and previously announced nothing; it renders text rather
than a mark, so it does not go through iconlink. Iconlink is now simpler than
before this branch - it delegates the decision rather than computing it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The only review comment is a non-blocking JSDoc nit.

Pull request overview

Improves accessibility by making redundant icons decorative and announcing links that open in a new tab.

Changes:

  • Adds shared NewTabCue support for external links.
  • Applies decorative alt text to repeated network icons.
  • Adds new-tab announcements to portal and Explorer links.
File summaries
File Reviewed change
components/Home/components/SectionBioNetworkAtlases/sectionBioNetworkAtlases.tsx Makes atlas marks decorative.
components/HCABioNetworks/Network/Atlas/components/SourceDatasets/components/MainColumn/components/table/viewBuilder.tsx Adds the new-tab cue to Explorer links.
components/common/Table/components/Cell/components/BioNetworkCell/bioNetworkCell.tsx Makes table marks decorative.
components/common/NewTabCue/types.ts Defines NewTabCue types.
components/common/NewTabCue/newTabCue.tsx Implements shared new-tab cue behavior.
components/common/NewTabCue/constants.ts Defines cue constants.
components/common/IconLink/iconLink.tsx Adds decorative image treatment and new-tab cues.
components/common/Hero/components/Title/components/HeroTitle/heroTitle.tsx Makes the hero mark decorative.
Review details

Suppressed comments (1)

components/HCABioNetworks/Network/Atlas/components/SourceDatasets/components/MainColumn/components/table/viewBuilder.tsx:87

  • The renderer now adds <NewTabCue> to the link label, so the JSDoc's “Text only” description is no longer accurate and could mislead callers about the accessible name. Please update that sentence to mention the visually hidden new-tab cue.
      <Link
        label={
          <>
            {HCA_EXPLORER_LABEL}
            <NewTabCue url={url} />
  • Files reviewed: 8/8 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@NoopDog NoopDog left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Code review (high effort). No correctness bug found: the NewTabCue predicate is an exact match for findable-ui Link's branch order (isURLObjectWithHrefAndQuery -> client-side -> isValidUrl -> plain Typography), including the invalid/empty-URL case where Link renders no anchor and the cue correctly renders nothing; label: ReactNode accepts the fragments; StaticImageProps.alt is a plain required string, so alt="" type-checks and emits a correctly decorative <img alt="">; visuallyHidden is a real export of the installed @mui/utils@7.3.11; and both IconLink consumers keep visible link text (StyledIconLink only overrides font), so no link is left unnamed by the alt="" change.

Five findings, all quality-level. Four are inline. Two items could not be anchored to a diff line:

Low - components/HCABioNetworks/Network/components/common/Tabs/tabs.tsx:53 (file not in this PR, so not inline): a missed instance of exactly the defect this PR fixes. <StaticImage alt="BICCN" ... /> is the icon of a tab whose label is "BICCN Publications", so the tab announces "BICCN BICCN Publications". The PR description asserts the audit found the BICCN graphic has no adjacent text repeating it; this one does.

On the verification claim (not line-anchorable): "cues missing on _blank links 0" holds only for the surfaces touched here. Four explicit-_blank Links still announce nothing - viewModelBuilders.tsx:509 (project title), components/Home/components/SectionHero/sectionHero.tsx:41, components/Home/components/SectionStayUpToDate/sectionStayUpToDate.tsx:31, and fileNameCell.tsx:60 - plus the social-media and footer entries in site-config/data-portal/dev/. So the cue is currently inconsistent site-wide; worth either scoping that claim in the description or filing a follow-up.

* @returns the cue, or null when the link stays in the same tab.
*/
export const NewTabCue = ({ url }: Props): JSX.Element | null => {
if (isClientSideNavigation(url) || !isValidUrl(url)) return null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Medium. The cue is derived from the URL alone and ignores an explicit target, while findable-ui's Link lets target override its default (target || ANCHOR_TARGET.BLANK / SELF).

No live defect today - neither IconLink nor the primary-data Link passes target. But the repo already has four Links that set target={ANCHOR_TARGET.BLANK} explicitly (viewModelBuilders.tsx:509, sectionHero.tsx:41, sectionStayUpToDate.tsx:31, fileNameCell.tsx:60), and the social/footer config adds more. The next maintainer reusing NewTabCue on an internal URL forced to _blank gets silence, and on an external URL forced to _self gets a cue that lies about where the link opens.

An optional target prop would close the gap: BLANK -> cue, SELF -> null, otherwise derive as now.

@@ -0,0 +1,3 @@
// Appended to a link's accessible name when it opens in a new tab. Leading
// space so it reads as a continuation of the label it follows.
export const NEW_TAB_LABEL = " (opens in a new tab)";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Low. This duplicates the existing EXTERNAL_LINK_TITLE = "(opens in a new tab)" at components/HCABioNetworks/Network/Atlas/components/SourceDatasets/components/MainColumn/components/table/components/FileNameCell/constants.ts:6.

Two constants for one user-facing string, rendered on the same pages, will drift the moment the wording is revised. This new shared module is the natural home for both - FileNameCell can import the text from here (its titleAccess wants the variant without the leading space, so export both, or trim at the use site).

*/
export const NewTabCue = ({ url }: Props): JSX.Element | null => {
if (isClientSideNavigation(url) || !isValidUrl(url)) return null;
return <span style={visuallyHidden}>{NEW_TAB_LABEL}</span>;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Low. style={visuallyHidden} is the only inline style prop in components/, views/ and pages/ - a grep for style={ across all three returns nothing else. The repo consistently uses a sibling *.styles.ts styled component instead.

A StyledVisuallyHidden span in newTabCue.styles.ts (spreading visuallyHidden) would match convention and keep the style overridable by consumers.

return function renderPrimaryData(ctx) {
const { hcaProjectId } = ctx.row.original;
if (!hcaProjectId) return null;
const url = buildHCADataExplorerProjectUrl(browserUrl, hcaProjectId);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Low. (Anchored here; the text in question is lines 69-70, just outside this hunk.) The buildRenderPrimaryData JSDoc still says "Text only - Link sends external URLs to a new tab with noopener/noreferrer", which this change makes false: the label is no longer text only, it now carries the NewTabCue element. Worth rewording to say the label carries the visually-hidden new-tab cue.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: analysis portal links give no 'opens in a new tab' cue and duplicate their icon's alt text

3 participants