Skip to content

Fix: notification links must have discernible text (a11y link-name)#128

Open
ZebaAfiaShama wants to merge 2 commits into
WPDevelopers:masterfrom
ZebaAfiaShama:fix/notificationx-link-discernible-text
Open

Fix: notification links must have discernible text (a11y link-name)#128
ZebaAfiaShama wants to merge 2 commits into
WPDevelopers:masterfrom
ZebaAfiaShama:fix/notificationx-link-discernible-text

Conversation

@ZebaAfiaShama

@ZebaAfiaShama ZebaAfiaShama commented Jun 18, 2026

Copy link
Copy Markdown

Problem

During an accessibility audit (Lighthouse / axe link-name), NotificationX links fail with "Links must have discernible text."

Reported failing element:

<a class="notificationx-link" href="https://…/product/exhibz-theme/" target="_blank"></a>

The anchor has an empty accessible name, so screen readers and accessibility-tree validation can't interpret the link.

Root cause

In frontend/core/Analytics.tsx, the a.notificationx-link element's only text-bearing child is the optional button text:

{ config.link_text ? link_text : '' } {children}   // press_bar branch
{ config.link_button ? link_text : '' } {children} // default branch

When a link is configured (link set, link_type !== 'none') but the link text is hidden/empty (link_text/link_button off, or link_button_text === '') and no button icon is shown, every child resolves to empty/whitespace → empty <a> → empty accessible name. There is no aria-label/title fallback. The notification content (image/title) is a sibling component, so it doesn't name this anchor.

The announcement-theme link button (frontend/themes/helpers/Button.js, themes 13–15) has the same pattern — its only child is announcement_link_button_text.

Fix

Add an aria-label fallback — configured button text → notification title → destination URL — applied only when no visible text is rendered (so links that already have visible text/icons are untouched and not overridden):

  • Analytics.tsx: both anchor branches (press_bar + default); also mark the decorative press-bar button icon alt="".
  • Button.js: announcement-theme anchor.

Since the anchor only renders when a destination exists, the accessible name is now never empty.

Verification

Validated the exact anchor expression with axe-core (the engine Lighthouse uses) against the failing config (link set, link_text:false, link_button_text:""):

Before After
Chrome a11y-tree name "" populated
axe link-name ❌ violation ✅ passes

Notes

  • Source-only change — the built assets/public/js/frontend.js needs the standard frontend build to regenerate.
  • Affects the free plugin's renderer (all front-end notifications, including Pro setups, render through it), so no Pro-side change is required.

Update: compiled bundle included

The runtime loads the prebuilt assets/public/js/frontend.js, so the source change has no effect until that bundle is rebuilt. This branch now also commits the same aria-label fix compiled into frontend.js (both a.notificationx-link anchors), so it works as soon as the branch is installed. If CI rebuilds the bundle on merge, that commit can be dropped — but please confirm the build actually emits (the current admin @types/react type errors block emit unless built with locked deps or TSC_COMPILE_ON_ERROR=true).

ZebaAfiaShama and others added 2 commits June 18, 2026 17:50
The frontend link (a.notificationx-link) and the announcement-theme link
button rendered with no text when a link is configured but the button text
is hidden/empty and no icon is shown, producing an empty accessible name.
This fails the axe/Lighthouse "link-name" rule ("Links must have discernible
text") and leaves the link unlabeled for screen readers and a11y-tree checks.

Add an aria-label fallback (configured button text -> notification title ->
destination URL) on both Analytics.tsx anchor branches (press_bar and
default) and the announcement Button.js anchor, applied only when no visible
text is rendered. Also mark the decorative press-bar button icon with alt="".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The runtime loads the prebuilt assets/public/js/frontend.js, so the source
change alone (Analytics.tsx / Button.js) has no effect until the bundle is
rebuilt. Apply the same aria-label fallback (button text -> title -> URL) to
the a.notificationx-link anchors in the compiled bundle so the fix is live
when this branch is installed as a plugin.

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

ShahrearMSf commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

✅ Found okay

Field Value
PR WPDevelopers/notificationx #128 — Fix: notification links must have discernible text (a11y link-name)
Branch / commits fix/notificationx-link-discernible-text — 2 commits: b2a74cb8 (source) + 8136733e (compiled bundle hand-patch)
Base master @ 6fd4baf9
Stat +24 / −1 across 3 files: Analytics.tsx, Button.js, assets/public/js/frontend.js

1. Source-side review — ✅ logic is sound

Analytics.tsx adds a single computed accessibleLabel reused by both anchor branches:

const isLinkTextRendered = !!(
    (config.source === 'press_bar' ? config.link_text : config.link_button) &&
    link_text && String(link_text).trim()
);
const accessibleLabel = isLinkTextRendered
    ? undefined
    : (link_text || config?.title || link || undefined);
  • Toggle resolution correctly diverges by source: press_bar uses config.link_text, default uses config.link_button. Verified by reading Analytics.tsx lines 105-145 — matches the actual JSX that controls visible text rendering further below (config.link_text ? link_text : '' for press_bar; config.link_button ? link_text : '' for default).
  • Whitespace-only strings correctly treated as "not rendered" via String(link_text).trim().
  • Fallback chain button-text → notification title → destination URL → undefined is the right order. URL last because it reads worst aloud (e.g. https://example.com/product/exhibz-theme/); title and configured text are preferred.
  • undefined (not empty string) is passed when text is already visible — React strips the attribute, so no aria-label shadowing of visible text. Avoids the WCAG 2.5.3 "label in name" risk.
  • alt="" on the decorative button-icon <img> is correct (the icon is purely visual; the anchor name comes from elsewhere).

Button.js (announcement themes 13-15) gets the same pattern, tighter:

const accessibleLabel =
    (announcement_link_button_text && String(announcement_link_button_text).trim())
        ? undefined
        : (config?.title || link || undefined);

Correct — these themes only have button text (no separate body), so the title-then-link fallback is sufficient.

Anchor render-guard check. PR body says "the anchor only renders when a destination exists". Confirmed by reading the surrounding JSX: both anchors are wrapped in conditions that require link truthy, so link || undefined at the tail of the fallback will always resolve to a non-empty string in practice. Defensively keeping the || undefined is the right call regardless.

2. Compiled-bundle review — ✅ patches at the correct call sites, ⚠️ guard semantics simplified

Diffed master vs PR-128 assets/public/js/frontend.js:

Metric master PR-128
Bundle size 1,277,844 bytes 1,277,157 bytes
aria-label occurrences 21 23 (+2)

Identified the 2 net-new sites in the minified bundle:

// Site 1 — press_bar branch (note button_icon check immediately after)
"…onClick:fn,style:R,\"aria-label\":d||(null==t?void 0:t.title)||u},h),(t?.button_icon)&&\"none\"!==…"

// Site 2 — default branch (note link_button check immediately after)
"…onClick:fn,\"aria-label\":d||(null==t?void 0:t.title)||u},i),t.link_button?d:\"\",\" \",M),…yt_channel_…"

Both sites are inside the Analytics-shaped IIFE with the exact context markers that match the two source branches. ✅ Right call sites. Fallback expression d || (t?.title) || u decodes to link_text || config?.title || link — matches the source fallback. ✅

⚠️ Source/bundle semantic divergence (worth a question)

The source uses a conditional:

const accessibleLabel = isLinkTextRendered ? undefined : (link_text || title || link);

The compiled bundle drops the isLinkTextRendered guard and just inlines the fallback unconditionally:

"aria-label": d || (t?.title) || u

Behaviour difference:

Case Source emits Bundle emits A11y impact
Button text rendered (toggle on, text non-empty) no aria-label aria-label="<button text>" (same as visible) None — values match. Mildly redundant.
Button text hidden (toggle off OR text empty) aria-label="<title|url>" aria-label="<title|url>" Identical

Why it diverged: PR body says the bundle commit was hand-patched, not a full rebuild — "If CI rebuilds the bundle on merge, that commit can be dropped." The hand-patch is a slightly simplified version. Functionally fine, but the bundle and source aren't byte-for-byte equivalent.

Recommendation: rebuild from source on merge to keep bundle ≡ source.

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.

2 participants