Skip to content

Navigation Menu

Sign in
Sign up

fix(core): render plain Link anchors inline so an ancestor Text clamp can truncate them #6090

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

Open
parinith-web wants to merge 5 commits into facebook:main
base: main
Choose a base branch
Loading
from parinith-web:fix-link-anchor-clamp-block-focus
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/link-anchor-clamp.md
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@astryxdesign/core': patch
---

[fix] Render a plain Link anchor as `inline` so an ancestor `<Text maxLines>` clamp can truncate it; flex layout is kept for the external-link icon and button forms, and Link/Text docs now cover the external-link clamp limitation. (#6021)
@ManoharPaturi
21 changes: 21 additions & 0 deletions packages/core/src/Link/Link.doc.mjs
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ export const docs = {
description: 'Maximum lines before truncation.',
default: '0',
},
{
name: 'hasBlockChild',
type: 'boolean',
description:
'Set when children render a block-level element (e.g. an HStack composing an icon with the label), so the Link keeps its inline-flex root and stays visibly focused. Trade-off: an ancestor <Text maxLines> can no longer truncate it — pass `maxLines` to the Link itself instead.',
default: 'false',
},
{
name: 'children',
type: 'ReactNode',
Expand Down Expand Up @@ -169,10 +176,14 @@ export const docs = {
bestPractices: [
{ guidance: true, description: 'Write descriptive, concise link text that clearly communicates the destination.' },
{ guidance: true, description: 'Set `isStandalone` when the link appears outside of inline text, so it receives proper base font sizing.' },
{ guidance: true, description: 'Let a plain Link be truncated by an ancestor <Text maxLines>; the anchor participates in the surrounding line boxes.' },
{ guidance: true, description: 'Only set `label` when the link content is not descriptive text (e.g. an icon-only link). For text links, the visible text is already the accessible name; adding `label` overrides it for screen readers, which is harmful.' },
{ guidance: false, description: 'Use Link for actions that do not navigate; use a Button instead.' },
{ guidance: false, description: 'Use generic text like "click here" or "read more"; describe the destination.' },
{ guidance: false, description: 'Set `label` on text links; `aria-label` prevents assistive technology from reading the actual link content.' },
{ guidance: true, description: 'Set `hasBlockChild` when Link children render a block-level element (e.g. an HStack composing an icon with the label), so keyboard focus stays visible.' },
{ guidance: false, description: 'Expect an ancestor <Text maxLines> to truncate an external Link. Its external-link icon uses an inline-flex layout, which an ancestor clamp cannot reach — pass `maxLines` to the Link itself.' },
{ guidance: false, description: 'Expect an ancestor <Text maxLines> to truncate a Link with `hasBlockChild` set. Its block-level content needs the inline-flex root, which an ancestor clamp cannot reach — pass `maxLines` to the Link itself.' },
],
anatomy: [
{name: 'Label', required: true, description: 'The visible text of the link.'},
Expand Down Expand Up @@ -301,10 +312,14 @@ export const docsZh = {
bestPractices: [
{ guidance: true, description: 'Write descriptive, concise link text that clearly communicates the destination.' },
{ guidance: true, description: 'Set `isStandalone` when the link appears outside of inline text, so it receives proper base font sizing.' },
{ guidance: true, description: 'Let a plain Link be truncated by an ancestor <Text maxLines>; the anchor participates in the surrounding line boxes.' },
{ guidance: true, description: 'Only set `label` when the link content is not descriptive text (e.g. an icon-only link). For text links, the visible text is already the accessible name; adding `label` overrides it for screen readers, which is harmful.' },
{ guidance: false, description: 'Use Link for actions that do not navigate; use a Button instead.' },
{ guidance: false, description: 'Use generic text like "click here" or "read more"; describe the destination.' },
{ guidance: false, description: 'Set `label` on text links; `aria-label` prevents assistive technology from reading the actual link content.' },
{ guidance: true, description: 'Set `hasBlockChild` when Link children render a block-level element (e.g. an HStack composing an icon with the label), so keyboard focus stays visible.' },
{ guidance: false, description: 'Expect an ancestor <Text maxLines> to truncate an external Link. Its external-link icon uses an inline-flex layout, which an ancestor clamp cannot reach — pass `maxLines` to the Link itself.' },
{ guidance: false, description: 'Expect an ancestor <Text maxLines> to truncate a Link with `hasBlockChild` set. Its block-level content needs the inline-flex root, which an ancestor clamp cannot reach — pass `maxLines` to the Link itself.' },
],
anatomy: [
{name: 'Label', required: true, description: 'The visible text of the link.'},
Expand All @@ -324,10 +339,14 @@ export const docsDense = {
bestPractices: [
{ guidance: true, description: 'Write descriptive, concise link text that clearly communicates the destination.' },
{ guidance: true, description: 'Set `isStandalone` when the link appears outside of inline text, so it receives proper base font sizing.' },
{ guidance: true, description: 'Let a plain Link be truncated by an ancestor <Text maxLines>; the anchor participates in the surrounding line boxes.' },
{ guidance: true, description: 'Only set `label` when the link content is not descriptive text (e.g. an icon-only link). For text links, the visible text is already the accessible name; adding `label` overrides it for screen readers, which is harmful.' },
{ guidance: false, description: 'Use Link for actions that do not navigate; use a Button instead.' },
{ guidance: false, description: 'Use generic text like "click here" or "read more"; describe the destination.' },
{ guidance: false, description: 'Set `label` on text links; `aria-label` prevents assistive technology from reading the actual link content.' },
{ guidance: true, description: 'Set `hasBlockChild` when Link children render a block-level element (e.g. an HStack composing an icon with the label), so keyboard focus stays visible.' },
{ guidance: false, description: 'Expect an ancestor <Text maxLines> to truncate an external Link. Its external-link icon uses an inline-flex layout, which an ancestor clamp cannot reach — pass `maxLines` to the Link itself.' },
{ guidance: false, description: 'Expect an ancestor <Text maxLines> to truncate a Link with `hasBlockChild` set. Its block-level content needs the inline-flex root, which an ancestor clamp cannot reach — pass `maxLines` to the Link itself.' },
],
anatomy: [
{name: 'Label', required: true, description: 'The visible text of the link.'},
Expand Down Expand Up @@ -355,6 +374,8 @@ export const docsDense = {
onClick: 'Click event handler',
tooltip: 'Tooltip text on hover',
isStandalone: 'Applies base font sizing',
hasBlockChild:
'Set when children render a block-level element (e.g. HStack + icon), so the Link keeps its inline-flex root and stays visibly focused. Trade-off: an ancestor maxLines clamp can no longer reach it.',
children: 'Link content',
},
},
Expand Down
149 changes: 149 additions & 0 deletions packages/core/src/Link/Link.test.tsx
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import {fireEvent, render, screen} from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import {Link} from './Link';
import {LinkProvider} from './LinkProvider';
import {HStack} from '../HStack';
import {Icon} from '../Icon';
import {Text} from '../Text';

function CustomLink({
children,
Expand Down Expand Up @@ -415,3 +418,149 @@ describe('Link', () => {
expect(link.className).toContain('secondary');
});
});

// =============================================================================
// Display: an ancestor clamp (<Text maxLines>) can only truncate an anchor
// that participates in the surrounding line boxes, so the plain anchor must
// be `inline`; flex layout is reserved for the icon/button forms (#6021).
// =============================================================================

/**
* Extracts class-name tokens from a CSS selector (e.g. the selectorText of
* a CSSStyleRule), so callers can match against an element's classList by
* exact token rather than by substring. Handles compound selectors
* (`.a.b:hover`) and comma-separated selector lists (`.a, .b`).
*/
function classTokensFromSelector(selectorText: string): string[] {
const tokens: string[] = [];
const classRegex = /\.([-_a-zA-Z0-9]+)/g;
let match: RegExpExecArray | null;
while ((match = classRegex.exec(selectorText)) !== null) {
tokens.push(match[1]);
}
return tokens;
}

/**
* Recursively collects every CSSStyleRule from a rule list, descending into
* grouping rules (e.g. `@media`) so declarations nested in a condition are
* not missed.
*/
function collectStyleRules(rules: CSSRuleList, out: CSSStyleRule[]): void {
for (const rule of Array.from(rules)) {
if (rule instanceof CSSStyleRule) {
out.push(rule);
} else if ('cssRules' in rule && rule.cssRules) {
collectStyleRules((rule as CSSGroupingRule).cssRules, out);
}
}
}

/**
* Returns the `display` value of every injected CSS rule that targets `el`,
* matched via the CSSOM (real selector tokens + real class list) rather than
* substring checks on raw selector/body text — a hashed atomic class like
* `x1a` can be a substring of an unrelated class like `x1a2b3c4`, so
* string-based matching can both false-positive and false-negative.
*/
function displayDeclarationsFor(el: Element): string[] {
const classes = new Set(Array.from(el.classList));
const styleRules: CSSStyleRule[] = [];
for (const sheet of Array.from(document.styleSheets)) {
let rules: CSSRuleList;
try {
rules = sheet.cssRules;
} catch {
continue;
}
collectStyleRules(rules, styleRules);
}

const declarations: string[] = [];
for (const rule of styleRules) {
const matchesElement = classTokensFromSelector(rule.selectorText).some(
token => classes.has(token),
);
if (!matchesElement) {
continue;
}
const display = rule.style.getPropertyValue('display');
if (display) {
declarations.push(display.trim());
}
}
return declarations;
}

describe('Link display', () => {
it('renders a plain anchor inline so an ancestor clamp can reach it', () => {
const {container} = render(<Link href="/docs">Documentation</Link>);
const anchor = container.querySelector('a')!;
expect(displayDeclarationsFor(anchor)).toContain('inline');
expect(displayDeclarationsFor(anchor)).not.toContain('inline-flex');
});

it('keeps flex layout for the external-link icon form', () => {
const {container} = render(
<Link href="https://example.com" isExternalLink>
External docs
</Link>,
);
const anchor = container.querySelector('a')!;
expect(displayDeclarationsFor(anchor)).toContain('inline-flex');
});

it('keeps the flex root (visible focus box) for display="block" links', () => {
const {container} = render(
<Link href="/docs" display="block">
Documentation
</Link>,
);
expect(displayDeclarationsFor(container.querySelector('a')!)).toContain(
'inline-flex',
);
});

it('keeps the flex root for links clamping themselves via maxLines', () => {
const {container} = render(
<Link href="/docs" maxLines={2}>
Documentation
</Link>,
);
expect(displayDeclarationsFor(container.querySelector('a')!)).toContain(
'inline-flex',
);
});

it('keeps flex layout for the button-rendered form', () => {
const {container} = render(
<Link onClick={() => {}} role="button">
Action
</Link>,
);
const button = container.querySelector('button')!;
expect(displayDeclarationsFor(button)).toContain('inline-flex');
});

it('keeps the flex root (visible focus box) when hasBlockChild flags a composed child like HStack + Icon', () => {
const {container} = render(
<Link href="/docs" hasBlockChild>
<HStack gap={1}>
<Icon icon="externalLink" size="xsm" />
<Text>Documentation</Text>
</HStack>
</Link>,
);
const anchor = container.querySelector('a')!;
expect(displayDeclarationsFor(anchor)).toContain('inline-flex');
});

it('stays inline for a plain-text Link so an ancestor clamp still reaches it', () => {
const {container} = render(
<Link href="/docs">Plain text, no composed children</Link>,
);
const anchor = container.querySelector('a')!;
expect(displayDeclarationsFor(anchor)).toContain('inline');
expect(displayDeclarationsFor(anchor)).not.toContain('inline-flex');
});
});
54 changes: 49 additions & 5 deletions packages/core/src/Link/Link.tsx
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,12 @@ import {useTranslator} from '../i18n';
*/
const styles = stylex.create({
base: {
display: 'inline-flex',
alignItems: 'center',
gap: spacingVars['--spacing-0-5'],
// `inline` (not `inline-flex`) so the anchor participates in the
// surrounding line boxes and an ancestor clamp (e.g. <Text maxLines>)
// can truncate it. An inline-flex box establishes its own formatting
// context, which an ancestor -webkit-line-clamp cannot reach —
// <Text maxLines={2}><Link>...</Link></Text> silently did nothing.
display: 'inline',
fontFamily: 'inherit',
fontSize: 'inherit',
lineHeight: 'inherit',
Expand Down Expand Up @@ -84,6 +87,17 @@ const styles = stylex.create({
pointerEvents: 'auto',
position: 'relative',
},
/**
* Flex layout for the cases that need it: external links append an icon
* after the text (icon centering + gap), and the button-rendered form
* keeps its previous inline-flex layout. On plain anchors this is
* deliberately NOT applied — see the `base` display note.
*/
flexLayout: {
display: 'inline-flex',
alignItems: 'center',
gap: spacingVars['--spacing-0-5'],
},
hasUnderline: {
textDecoration: 'underline',
},
Expand Down Expand Up @@ -256,6 +270,19 @@ export interface LinkProps extends BaseProps<
* @default 0
*/
maxLines?: number;
/**
* Set this when `children` renders a block-level element — for example a
* layout primitive like `HStack`/`VStack` composing an icon with the
* label. A plain anchor is `inline` so an ancestor `<Text maxLines>` clamp
* can truncate it, but an inline anchor wrapping a block-level child
* computes a focus outline that paints nothing in Chromium. Setting this
* keeps the Link's inline-flex root so keyboard focus stays visible; the
* trade-off is that an ancestor clamp can no longer truncate it (same
* limitation as `isExternalLink` — pass `maxLines` to the Link itself
* instead).
* @default false
*/
hasBlockChild?: boolean;
/**
* Link content (required).
*/
Expand Down Expand Up @@ -307,6 +334,7 @@ export function Link({
color = 'accent',
display = 'inline',
maxLines = 0,
hasBlockChild = false,
children,
rel: relFromProps,
xstyle,
Expand All @@ -329,6 +357,19 @@ export function Link({
// render as a <button> with link styling for semantic correctness.
const renderAsButton =
role === 'button' || (role === 'inert' && href == null);
const isExternalWithIcon = isExternalLink && !renderAsButton;
// A Link wrapping plain text has no block-level descendant, so it can stay
// `inline` and let an ancestor clamp (<Text maxLines>) truncate it — that
// is the composition this PR fixes. But a Link that establishes its own
// box (`display="block"`, or clamping itself via `maxLines`, or carrying
// the external-link icon) — or one the caller has told us wraps a
// block-level child via `hasBlockChild` (e.g. an HStack laying out an
// icon alongside the label) — must keep the inline-flex root: an inline
// anchor wrapping a block-level descendant computes a focus outline that
// paints nothing in Chromium, losing keyboard focus visibility on those
// forms.
const needsRootBox =
isExternalWithIcon || display !== 'inline' || maxLines > 0 || hasBlockChild;

const sharedContent = (
<>
Expand All @@ -341,7 +382,7 @@ export function Link({
maxLines={maxLines}>
{children}
</Text>
{isExternalLink && !renderAsButton && (
{isExternalWithIcon && (
<>
<Icon icon="externalLink" size="xsm" color="inherit" />
<VisuallyHidden>{newTabLabel}</VisuallyHidden>
Expand All @@ -366,6 +407,7 @@ export function Link({
themeProps('link', {color}),
focusOutlineProps.focusVisible(
styles.base,
styles.flexLayout,
styles.buttonReset,
linkColorStyles[color],
hasUnderline && styles.hasUnderline,
Expand Down Expand Up @@ -398,10 +440,11 @@ export function Link({
themeProps('link', {color}),
focusOutlineProps.focusVisible(
styles.base,
needsRootBox && styles.flexLayout,
linkColorStyles[color],
hasUnderline && styles.hasUnderline,
isStandalone && styles.standalone,
styles.disabled,
isDisabled && styles.disabled,
xstyle,
),
className,
Expand All @@ -426,6 +469,7 @@ export function Link({
themeProps('link', {color}),
focusOutlineProps.focusVisible(
styles.base,
needsRootBox && styles.flexLayout,
linkColorStyles[color],
hasUnderline && styles.hasUnderline,
isStandalone && styles.standalone,
Expand Down
14 changes: 14 additions & 0 deletions packages/core/src/Text/Text.doc.mjs
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,11 @@ export const docs = {
description:
'Use Text for headings; use Heading with a `level` prop (1\u20136) for section titles and headings.',
},
{
guidance: false,
description:
'Expect `maxLines` to truncate an external `Link` child. Its inline-flex icon layout escapes an ancestor clamp; give the Link its own `maxLines` (a plain Link child is clamped fine).',
},
],
},
};
Expand Down Expand Up @@ -266,6 +271,11 @@ export const docsZh = {
description:
'Use Text for headings; use Heading with a `level` prop (1\u20136) for section titles and headings.',
},
{
guidance: false,
description:
'Expect `maxLines` to truncate an external `Link` child. Its inline-flex icon layout escapes an ancestor clamp; give the Link its own `maxLines` (a plain Link child is clamped fine).',
},
],
},
};
Expand Down Expand Up @@ -310,6 +320,10 @@ export const docsDense = {
guidance: false,
description: 'Raw <p>/<h1>/<span>; use Text/Heading for theme tokens.',
},
{
guidance: false,
description: 'maxLines cannot truncate an external Link child; give the Link its own maxLines.',
},
{
guidance: false,
description:
Expand Down

AltStyle によって変換されたページ (->オリジナル) /