Skip to content

feat: add dir="auto" and text-align: justify for RTL support #2823

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.

Sign up for GitHub

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
ssmns wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add dir="auto" and text-align: justify for RTL support #2823

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 4 additions & 2 deletions src/common-elements/headers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,19 @@ export const H1 = styled.h1`
${extensionsHook('H1')};
`;

export const H2 = styled.h2`
export const H2 = styled.h2.attrs({ dir: 'auto' })`
${headerCommonMixin(2)};
color: ${({ theme }) => theme.colors.text.primary};
margin: 0 0 20px;
text-align: justify;

@displague displague Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The dir: 'auto' on line 23 is good. This text-align: justify I would drop, along with its twin on H3 and the two in Markdown/styled.elements.tsx.

Justification is orthogonal to direction — dir already solves RTL. This line changes heading alignment for every Redoc deployment, LTR included, as a side effect of an RTL PR. On headings specifically it does nothing in the usual single-line case and stretches wrapped headings unevenly.

If the intent is alignment that follows the text direction, the logical property does it properly and is a no-op for existing LTR users:

text-align: start;

Either way I would move alignment into its own PR — it is a visual/typographic decision maintainers will want to weigh separately, and it would be a shame to have it hold up the dir="auto" work, which is much less contentious.


${extensionsHook('H2')};
`;

export const H3 = styled.h2`
export const H3 = styled.h2.attrs({ dir: 'auto' })`
${headerCommonMixin(3)};
color: ${({ theme }) => theme.colors.text.primary};
text-align: justify;

${extensionsHook('H3')};
`;
Expand Down
2 changes: 1 addition & 1 deletion src/components/ApiInfo/ApiInfo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export class ApiInfo extends React.Component<ApiInfoProps> {
{info.title} {version}
</ApiHeader>
{!hideDownloadButtons && (
<p>
<p dir="auto">
{l('downloadSpecification')}:
{downloadUrls?.map(({ title, url }) => {
return (
Expand Down
2 changes: 1 addition & 1 deletion src/components/ErrorBoundary.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class ErrorBoundary extends React.Component<
<ErrorWrapper>
<h1>Something went wrong...</h1>
<small> {this.state.error.message} </small>
<p>
<p dir="auto">

@displague displague Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor, and easy to miss: this <p> wraps <details><summary>Stack trace</summary><pre>{stack}</pre></details>.

dir="auto" infers direction from the first strong directional character in the subtree. Today that is "Stack trace", so you get LTR and everything is fine. But a stack trace is code, and code should be unconditionally LTR rather than inferred — if the surrounding content ever changes such that a strong RTL character comes first, the trace would render mirrored and become quite hard to read.

dir="ltr" on the <pre> (or on this <p>) expresses the intent more precisely. Same reasoning as keeping code out of machine translation in #2697: code is not prose and should not inherit prose's locale behaviour.

<details>
<summary>Stack trace</summary>
<pre>{this.state.error.stack}</pre>
Expand Down
7 changes: 7 additions & 0 deletions src/components/Markdown/SanitizedMdBlock.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ import styled from 'styled-components';
// Workaround for DOMPurify type issues (https://github.com/cure53/DOMPurify/issues/1034)
const dompurify = DOMPurify['default'] as DOMPurify.DOMPurify;

// Add dir="auto" to p and h2 elements for RTL support
dompurify.addHook('afterSanitizeAttributes', (node) => {

@displague displague Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is the cause of the 14 suite failures, and I think it needs a different home regardless.

It throws at import time. Line 10 above is const dompurify = DOMPurify['default'] as .... Under the repo's Jest/CommonJS interop, DOMPurify['default'] is undefined. That was previously harmless because dompurify was only dereferenced inside the sanitize helper, which is called at render time and only when the option is on. Moving a dompurify.addHook(...) call to module scope makes it execute on import:

TypeError: Cannot read properties of undefined (reading 'addHook')
  at src/components/Markdown/SanitizedMdBlock.tsx:13:11
  at src/components/Markdown/AdvancedMarkdown.tsx:5:1
  ...
  at src/components/__tests__/FieldDetails.test.tsx:4:1

Because almost everything imports Markdown transitively, 14 of 27 suites never load.

It is a global side effect. addHook mutates the shared DOMPurify instance, at import time, for the whole process. Redoc is embedded into other people's applications; if the host app also uses DOMPurify, merely importing Redoc would start adding dir="auto" to every <p> and <h2> the host sanitizes. The hook is also never removed and would accumulate across repeated module evaluation.

It only runs when sanitize is on, which defaults to false — see the main comment; that makes the markdown part of this feature inert for default configurations.

All three go away if you set the attribute during markdown rendering instead. In src/services/MarkdownRenderer.ts the renderer is already customised, so something along these lines:

const renderParagraph = renderer.paragraph.bind(renderer);
renderer.paragraph = (...args) => renderParagraph(...args).replace('<p>', '<p dir="auto">');

(or override renderer.heading similarly). No global state, no dependency on sanitize, and it works for the default configuration. I used this same pattern for renderer.code/renderer.codespan in #2697 and verified DOMPurify keeps the added attribute when sanitization is enabled.

if (node.tagName === 'P' || node.tagName === 'H2') {
node.setAttribute('dir', 'auto');
}
});

const StyledMarkdownSpan = styled(StyledMarkdownBlock)`
display: inline;
`;
Expand Down
3 changes: 3 additions & 0 deletions src/components/Markdown/styled.elements.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ export const StyledMarkdownBlock = styled(
line-height: ${props => props.theme.typography.lineHeight};

p {
text-align: justify;

@displague displague Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same point as on headers.ts, but this one worries me more: it justifies every paragraph of every description in the API docs.

Redoc's middle panel is narrow, and justified text in a narrow measure produces uneven word spacing and vertical rivers of whitespace. WCAG 1.4.8 explicitly advises against justified blocks of text because that irregular spacing is harder to track for readers with dyslexia and some low-vision conditions — which cuts against the spirit of the change.

Since this is applied to all users regardless of language, it is a global typographic change rather than RTL support. text-align: start would give you direction-aware alignment with no effect on LTR readers, if that is what you were after.


&:last-child {
margin-bottom: 0;
}
Expand Down Expand Up @@ -63,6 +65,7 @@ export const StyledMarkdownBlock = styled(
h2 {
${headerCommonMixin(2)};
color: ${props => props.theme.colors.text.primary};
text-align: justify;
}

code {
Expand Down
1 change: 1 addition & 0 deletions src/components/SearchBox/styled.elements.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const SearchWrap = styled.div`

export const SearchInput = styled.input.attrs(() => ({
className: 'search-input',
dir: 'auto',

@displague displague Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This one is just nice — no notes. An RTL user typing into the search field gets the caret and text alignment they expect, it costs one line, and there is no effect on LTR users. If the PR were trimmed down to only the dir="auto" additions, this is the piece I would keep first.

}))`
width: calc(100% - ${props => props.theme.spacing.unit * 8}px);
box-sizing: border-box;
Expand Down