feat(texteditor): add text alignment toggling buttons by habibayman · Pull Request #5677 · learningequality/studio

habibayman · 2026-01-30T15:15:01Z

Summary

This PR includes the addition of a toggle alignment (Left/Right) button.
From the motives of making the RTL editor experience better and give users the ability to seamlessly mix RTL & LTR text in the same editing block.

It includes support for all nodes & Marks we have:

For this to work with our backward compatible dual markdown conversion pipeline; I have altered the markdown serializer to wrap non-default direction nodes with HTML tags containing the align attribute set.

And of course, updated the unit tests accordingly.

References

Figma Design
currently this PR represents part of Update rich text editor to flexible, extensible rich text editing framework #5049
fixes [Rich Text Editor]: add a toggle text alignment (R/L) button #5385

Reviewer guidance

Try different combinations of nodes & marks and see if their alignment gets aligned correctly

Tip

You can use the checklist I wrote in the description section

Check that the alignment button's icon reflects direction correctly
Does everything persists on save?

Note

Check the native behavior of the extension from the official TipTap documentation to see where its limitations -if any- exist.
https://tiptap.dev/docs/editor/extensions/functionality/textalign

learning-equality-bot · 2026-01-30T15:15:37Z

👋 Thanks for contributing!

We will assign a reviewer within the next two weeks. In the meantime, please ensure that:

You ran pre-commit locally
All issue requirements are satisfied
The contribution is aligned with our Contributing guidelines. Pay extra attention to Using generative AI. Pull requests that don't follow the guidelines will be closed.

We'll be in touch! 😊

…toolbar

…gs with alignment dir

…g logic

…hildren by calling serializeChildren directly

…gnment extension

rtibblesbot

Good feature addition for RTL/LTR text alignment toggling. The serializer refactoring from mutation-based to return-based is a nice improvement. One bug in the overflow dropdown disabled logic.

CI passing.

Blocking:

Inverted :disabled logic on overflow align button (see inline)

Suggestions:

SVG icon dimension mismatch between align-left and align-right
Peer dependency version mismatch for @tiptap/extension-text-align

Nitpick:

Double space in image markdown output when no dimensions present

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Reviewed the pull request diff checking for:

Correctness: bugs, edge cases, undocumented behavior, resource leaks, hardcoded values
Design: unnecessary complexity, naming, readability, comment accuracy, redundant state
Architecture: duplicated concerns, minimal interfaces, composition over inheritance
Testing: behavior-based assertions, mocks only at hard boundaries, accurate coverage
Completeness: missing dependencies, unupdated usages, i18n, accessibility, security
Principles: DRY (same reason to change), SRP, Rule of Three (no premature abstraction)
Checked CI status and linked issue acceptance criteria
For UI changes: inspected screenshots for layout, visual completeness, and consistency

...contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/components/EditorToolbar.vue

contentcuration/contentcuration/frontend/shared/views/TipTapEditor/assets/icon-alignRight.svg

...ntcuration/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/utils/markdown.js

package.json

rtibblesbot · 2026-03-05T21:21:37Z

.../contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/utils/markdownSerializer.js

            }
          }
-          break;
+          return parts.join('\n\n');


praise: Nice refactoring of the serializer from mutation-based (result +=) to a return-based approach. This makes each case self-contained and much easier to reason about, especially now that alignment wrapping needs to inspect the serialized children before deciding the output format.

rtibblesbot · 2026-03-05T21:21:37Z

...tion/contentcuration/frontend/shared/views/TipTapEditor/__tests__/markdownSerializer.spec.js

+          type: 'paragraph',
+          content: [{ type: 'text', text: 'n', marks: [{ type: 'subscript' }] }],
+        },
+      ];


praise: Thorough test coverage for text alignment serialization — covers right/left/center/no-attr cases, headings, inline marks inside aligned paragraphs, and mixed alignment documents. Good edge case thinking.

...entcuration/frontend/shared/views/TipTapEditor/TipTapEditor/extensions/SmallTextExtension.js

…tton

rtibblesbot

Solid iteration — the prior blocking issue (inverted disabled logic) and the SVG dimension mismatch are both fixed. 3 prior finding(s) resolved.

CI passing. New strings target the default branch (unstable) — appropriate.

Suggestions:

serializeNode guard clause returns undefined, which the return-based refactor now concatenates (see inline)
Double space in no-dimensions image markdown path (prior nitpick, still present — see inline)

Note: the reviewer thread from @nucleogenesis / @radinamatic / @marcellamaki about the alignment icon not reflecting the browser's automatic bidi direction (e.g., Arabic auto-aligning right while the icon still shows left) is a real UX concern worth addressing, though it may be out of scope for this PR.

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Compared the current PR state against findings from a prior review:

Retrieved prior bot reviews deterministically via the GitHub API
Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
Only raised NEW findings for newly introduced code
Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
Checked CI status and linked issue acceptance criteria

.../contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/utils/markdownSerializer.js

...ntcuration/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/utils/markdown.js

...tion/contentcuration/frontend/shared/views/TipTapEditor/__tests__/markdownSerializer.spec.js

.../contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/utils/markdownSerializer.js

rtibblesbot

Good iteration — the bidi alignment detection via getEffectiveAlignment is a nice solution to the RTL concern raised by reviewers.

CI passing. New strings target unstable (default branch) — appropriate.

3 prior finding(s) resolved.

Suggestions:

serializeNode guard clause still returns undefined, which concatenates as the literal string "undefined" (UNADDRESSED — see inline)
Double space in no-dimensions image markdown path when alignment is present (UNADDRESSED — see inline)

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Compared the current PR state against findings from a prior review:

Retrieved prior bot reviews deterministically via the GitHub API
Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
Only raised NEW findings for newly introduced code
Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
Checked CI status and linked issue acceptance criteria

.../contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/utils/markdownSerializer.js

...ntcuration/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/utils/markdown.js

rtibblesbot · 2026-03-08T00:17:10Z

...entcuration/frontend/shared/views/TipTapEditor/TipTapEditor/composables/useToolbarActions.js

+    const dom = editorInstance.view.domAtPos(from).node;
+    const el = dom.nodeType === 1 ? dom : dom.parentElement;
+
+    return el ? window.getComputedStyle(el).textAlign : 'left';


praise: Smart fallback to getComputedStyle for detecting browser-resolved bidi alignment — this handles the RTL auto-alignment case that pure attribute checking would miss.

…kdown serializer

rtibblesbot

Good iteration — all prior findings are resolved. The serializer refactoring to return-based is clean and the bidi alignment detection via getEffectiveAlignment is a solid approach.

CI passing. New strings target unstable (default branch) — appropriate.

5 prior finding(s) resolved.

Suggestions:

getEffectiveAlignment missing null guard when called from alignAction computed (see inline)

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Compared the current PR state against findings from a prior review:

Retrieved prior bot reviews deterministically via the GitHub API
Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
Only raised NEW findings for newly introduced code
Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
Checked CI status and linked issue acceptance criteria

...entcuration/frontend/shared/views/TipTapEditor/TipTapEditor/composables/useToolbarActions.js

rtibblesbot · 2026-03-08T01:03:40Z

.../contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/utils/markdownSerializer.js


-    serializeNode(doc, null, 0);
-    return result.trim();
+    return serializeNode(doc, null, 0).trim();


praise: The refactor from mutation-based (result +=) to return-based serialization is a meaningful improvement — each node handler is now a pure function returning its output, which makes the control flow easier to reason about and eliminates the shared mutable state.

rtibblesbot

Good iteration — all prior findings are resolved. The bidi alignment detection via getEffectiveAlignment, the serializer alignment support, and the thorough test coverage make this a solid feature addition.

CI passing. PR targets unstable (default branch) — appropriate for new strings.

6 prior finding(s) resolved.

Suggestions:

MobileFormattingBar align button missing :is-available binding (see inline)

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Compared the current PR state against findings from a prior review:

Retrieved prior bot reviews deterministically via the GitHub API
Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
Only raised NEW findings for newly introduced code
Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
Checked CI status and linked issue acceptance criteria

rtibblesbot · 2026-03-11T17:22:50Z

...n/frontend/shared/views/TipTapEditor/TipTapEditor/components/toolbar/MobileFormattingBar.vue

+      <ToolbarButton
+        :title="alignAction.title"
+        :icon="alignAction.icon"
+        :is-active="alignAction.isActive"


suggestion: The align button here doesn't bind :is-available="alignAction.isAvailable", unlike the desktop EditorToolbar.vue (line 77). This means the align button won't be disabled inside code blocks on mobile. Consider adding :is-available="alignAction.isAvailable" for parity.

rtibblesbot · 2026-03-11T17:22:50Z

...entcuration/frontend/shared/views/TipTapEditor/TipTapEditor/composables/useToolbarActions.js

  const editor = inject('editor', null);

+  // helper
+  const getEffectiveAlignment = editorInstance => {


praise: getEffectiveAlignment is a clean solution to the bidi/RTL concern — checking isActive first for explicit alignment, then falling back to getComputedStyle for browser-resolved direction. The null guard at line 10 keeps it consistent with the rest of the composable's defensive patterns.

nucleogenesis

The latest changes look great in manual testing! Thanks @habibayman !

rtibblesbot's suggestions were addressed

nucleogenesis self-requested a review February 6, 2026 20:59

nucleogenesis self-assigned this Feb 6, 2026

nucleogenesis removed their request for review February 6, 2026 20:59

habibayman added 6 commits March 5, 2026 01:04

build: add tiptap text align dependency

4aebc0e

feat(texteditor): add basic toggle alignmen functionality

e54e72d

fix(texteditor): toolbar overflow logic after adding alignment button

baac181

fix(texteditor): disable toggling alignment for code blocks

d6b9621

feat(texteditor): add toggling text alignment button to touchscreens …

c270ce4

…toolbar

fix(texteditor): adjust image extension to account for node alignment

f2aa6e6

habibayman force-pushed the feat/RTE-text-align branch from 00187e5 to f2aa6e6 Compare March 4, 2026 23:05

habibayman added 5 commits March 5, 2026 03:24

feat(texteditor): adjust markdown serializer to wrap nodes in html ta…

74341c4

…gs with alignment dir

test(texteditor): cover markdown serializer text alignemnt serializin…

22a5c2d

…g logic

fix(texteditor): listItem serializer short-circuiting its paragraph c…

38a9805

…hildren by calling serializeChildren directly

fix(texteditor): include align in images dual saving syntax

c3d1cbe

feat(texteditor): adjust small text displaying to account for textAli…

2dd5003

…gnment extension

habibayman marked this pull request as ready for review March 5, 2026 02:16

nucleogenesis requested a review from rtibblesbot March 5, 2026 21:17

rtibblesbot previously requested changes Mar 5, 2026

View reviewed changes

nucleogenesis reviewed Mar 5, 2026

View reviewed changes

...entcuration/frontend/shared/views/TipTapEditor/TipTapEditor/extensions/SmallTextExtension.js Show resolved Hide resolved

habibayman added 2 commits March 8, 2026 00:45

fix(texteditor): inverted disabling condition for overflowed align bu…

f17bbf6

…tton

fix(texteditor): match dimensions for right align SVG with left align

f3bc749

rtibblesbot reviewed Mar 7, 2026

View reviewed changes

feat(texteditor): detect alignemnt of every node on cursor position

dbc30f2

rtibblesbot reviewed Mar 8, 2026

View reviewed changes

perf(texteditor): add a safeguard return string for the recursive mar…

c7cf5ee

…kdown serializer

rtibblesbot reviewed Mar 8, 2026

View reviewed changes

pef(texteditor): add a null guard return value for getEffectiveAlignment

42d8179

rtibblesbot reviewed Mar 11, 2026

View reviewed changes

nucleogenesis approved these changes Mar 11, 2026

View reviewed changes

nucleogenesis merged commit 6344d05 into learningequality:unstable Mar 11, 2026
15 checks passed

Conversation

habibayman commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

References

Reviewer guidance

Uh oh!

learning-equality-bot bot commented Jan 30, 2026

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rtibblesbot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rtibblesbot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rtibblesbot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

nucleogenesis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

habibayman commented Jan 30, 2026 •

edited

Loading