-
-
Notifications
You must be signed in to change notification settings - Fork 7k
feat: auto text direction (RTL) in composer editor and previews #1993
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
base: staging
Are you sure you want to change the base?
Changes from all commits
ceef100
179ba4d
fd487c8
940a49c
e2860a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -907,7 +907,11 @@ export const OnlyEditor = forwardRef< | |
| const editor = useEditor({ | ||
| extensions: [ | ||
| Document, | ||
| Paragraph, | ||
| Paragraph.configure({ | ||
| HTMLAttributes: { | ||
| dir: 'auto', | ||
| }, | ||
| }), | ||
| Text, | ||
| Underline, | ||
| Bold, | ||
|
Comment on lines
907
to
917
This comment was marked as outdated.
Sorry, something went wrong.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, confirmed: sanitizePostContent stripped the per paragraph dir attribute before storage. The impact was partial (the editor re applies dir=auto from the extension config and the preview wrappers carry their own dir=auto), but stored content lost per paragraph granularity and the public share page rendered without it. Fixed in 179ba4d by adding dir to ALLOWED_ATTR. |
||
|
|
@@ -1021,6 +1025,9 @@ export const OnlyEditor = forwardRef< | |
| ? [ | ||
| Heading.configure({ | ||
| levels: [1, 2, 3], | ||
| HTMLAttributes: { | ||
| dir: 'auto', | ||
| }, | ||
| }), | ||
| ] | ||
| : []), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,7 @@ export const InstagramPreview: FC<{ | |
|
|
||
| const finalValue = | ||
| `<strong class="text-[15px] font-[600]">${integration?.name} </strong>` + | ||
| `<span dir="auto">` + | ||
| newContent | ||
| .slice(start, end) | ||
| .replace(/\[\[\[([.\s\S]*?)]]]/, (match, match1) => { | ||
|
|
@@ -43,7 +44,7 @@ export const InstagramPreview: FC<{ | |
| newContent.slice(end).replace(/\[\[\[([.\s\S]*?)]]]/, (match, match1) => { | ||
| return `<span class="font-bold font-[arial]" style="color: #ae8afc">${match1}</span>`; | ||
| }) + | ||
| `</mark>`; | ||
| `</mark></span>`; | ||
|
|
||
| return { text: finalValue, images: p.image }; | ||
| }); | ||
|
|
@@ -82,6 +83,7 @@ export const InstagramPreview: FC<{ | |
| /> | ||
| )} | ||
| <div | ||
| dir="auto" | ||
| className="text-[14px] font-[400] whitespace-pre-line" | ||
| dangerouslySetInnerHTML={{ | ||
| __html: renderContent?.[0]?.text, | ||
|
Comment on lines
83
to
89
This comment was marked as outdated.
Sorry, something went wrong.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed with a minimal repro: the Latin username as first strong character forced the whole caption block LTR. Fixed in fd487c8 by wrapping the caption content in its own span with dir=auto, so the username stays inline (like on Instagram itself) while the content resolves its own direction. Verifying this also surfaced that the literal indexOf(' ') checks in strip.html.validation.ts and add.edit.modal.tsx missed paragraphs carrying the new dir attribute, which made stripHtmlValidation return raw HTML at publish time; fixed in the same commit by matching '<p' instead. |
||
|
|
@@ -181,6 +183,7 @@ export const InstagramPreview: FC<{ | |
| <div className="flex flex-col gap-[6px] flex-1"> | ||
| <div className="flex gap-[4px] py-[8px]"> | ||
| <div | ||
| dir="auto" | ||
| className="whitespace-pre-line text-[14px] font-[400] flex-1" | ||
| dangerouslySetInnerHTML={{ | ||
| __html: value.text, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ const ALLOWED_TAGS = [ | |
| ]; | ||
|
|
||
| const ALLOWED_ATTR = [ | ||
| 'dir', | ||
| 'href', | ||
| 'target', | ||
| 'rel', | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -179,17 +179,17 @@ export const stripHtmlValidation = ( | |
| return striptags( | ||
| convertMention( | ||
| value | ||
| .replace(/<h1>([.\s\S]*?)<\/h1>/g, (match, p1) => { | ||
| .replace(/<h1[^>]*>([.\s\S]*?)<\/h1>/g, (match, p1) => { | ||
| return `<h1># ${p1}</h1>\n`; | ||
| }) | ||
| .replace(/&/gi, '&') | ||
| .replace(/ /gi, ' ') | ||
| .replace(/"/gi, '"') | ||
| .replace(/'/gi, "'") | ||
| .replace(/<h2>([.\s\S]*?)<\/h2>/g, (match, p1) => { | ||
| .replace(/<h2[^>]*>([.\s\S]*?)<\/h2>/g, (match, p1) => { | ||
| return `<h2>## ${p1}</h2>\n`; | ||
| }) | ||
| .replace(/<h3>([.\s\S]*?)<\/h3>/g, (match, p1) => { | ||
| .replace(/<h3[^>]*>([.\s\S]*?)<\/h3>/g, (match, p1) => { | ||
| return `<h3>### ${p1}</h3>\n`; | ||
| }) | ||
| .replace(/<u>([.\s\S]*?)<\/u>/g, (match, p1) => { | ||
|
|
@@ -201,7 +201,7 @@ export const stripHtmlValidation = ( | |
| .replace(/<li.*?>([.\s\S]*?)<\/li.*?>/gm, (match, p1) => { | ||
| return `<li>- ${p1.replace(/\n/gm, '')}</li>`; | ||
| }) | ||
| .replace(/<p>([.\s\S]*?)<\/p>/g, (match, p1) => { | ||
| .replace(/<p[^>]*>([.\s\S]*?)<\/p>/g, (match, p1) => { | ||
| return `<p>${p1}</p>\n`; | ||
| }) | ||
| .replace( | ||
|
|
@@ -217,7 +217,7 @@ export const stripHtmlValidation = ( | |
| .replace(/</gi, '<'); | ||
| } | ||
|
|
||
| if (value.indexOf('<p>') === -1 && !none) { | ||
| if (!/<p[\s>]/i.test(value) && !none) { | ||
| return value; | ||
| } | ||
|
|
||
|
Comment on lines
217
to
223
This comment was marked as outdated.
Sorry, something went wrong.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed and reproduced: markdown mode returned the paragraphs concatenated with no newlines, and the headings also lost their # prefixes since h1-h3 now carry dir=auto too. Fixed in the markdown branch by making the p and h1/h2/h3 regexes attribute-tolerant ([^>]*). Verified the output is now identical for content with and without the dir attribute. |
||
|
|
||
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point, tightened to a tag-shaped check: /<p[\s>]/i in both add.edit.modal.tsx and the strip.html.validation.ts guard. It matches
and
but ignores a stray <p followed by other characters in plain text. Note the same false positive class already existed with the original indexOf('
') check for text containing a literal
, so this is now stricter than the original code as well.