Skip to content

Navigation Menu

Sign in
Sign up

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

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 3 commits into facebook:main
base: main
Choose a base branch
Loading
from parinith-web:fix-link-anchor-clamp
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
6 changes: 6 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 @@ -169,10 +169,12 @@ 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: 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.' },
],
anatomy: [
{name: 'Label', required: true, description: 'The visible text of the link.'},
Expand Down Expand Up @@ -301,10 +303,12 @@ 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: 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.' },
],
anatomy: [
{name: 'Label', required: true, description: 'The visible text of the link.'},
Expand All @@ -324,10 +328,12 @@ 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: 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.' },
],
anatomy: [
{name: 'Label', required: true, description: 'The visible text of the link.'},
Expand Down
102 changes: 102 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 @@ -415,3 +415,105 @@ 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 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');
});
});
28 changes: 23 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 @@ -329,6 +343,7 @@ 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;

const sharedContent = (
<>
Expand All @@ -341,7 +356,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 +381,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 +414,11 @@ export function Link({
themeProps('link', {color}),
focusOutlineProps.focusVisible(
styles.base,
isExternalWithIcon && styles.flexLayout,
linkColorStyles[color],
hasUnderline && styles.hasUnderline,
isStandalone && styles.standalone,
styles.disabled,
isDisabled && styles.disabled,
xstyle,
),
className,
Expand All @@ -426,6 +443,7 @@ export function Link({
themeProps('link', {color}),
focusOutlineProps.focusVisible(
styles.base,
isExternalWithIcon && 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 によって変換されたページ (->オリジナル) /