✨(frontend) enable justified text alignment#2294
Conversation
ericboucher
changed the title
ericboucher
changed the title
WalkthroughThis PR adds justified text alignment support to the Impress document editor. The frontend toolbar now includes a justify button positioned after the right-align button. The DOCX export system is updated to handle justified alignment through a new shared styling converter function ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/frontend/apps/impress/src/features/docs/doc-export/blocks-mapping/blockNoteDocxBlockProps.ts`:
- Around line 19-43: The current code conditionally omits the entire shading/run
objects when backgroundColor/textColor are 'default' or missing, but docx can
accept undefined subfields; simplify by always returning the shading object
(with type: ShadingType.CLEAR) and the run object while setting shading.fill to
colors[props.backgroundColor]?.background?.slice(1) (or undefined) and run.color
to colors[props.textColor]?.text?.slice(1) (or undefined), removing the outer
ternaries that produce undefined for the whole shading/run so invalid XML won’t
be produced and lookups that miss a color simply leave the subfield undefined;
update the code around the shading and run expressions (references: shading,
ShadingType.CLEAR, run, colors, props.backgroundColor, props.textColor)
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5cbbcbed-9d82-41f8-9bae-662d0d78575f
📒 Files selected for processing (8)
CHANGELOG.mdsrc/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolBar/BlockNoteToolbar.tsxsrc/frontend/apps/impress/src/features/docs/doc-export/blocks-mapping/blockNoteDocxBlockProps.tssrc/frontend/apps/impress/src/features/docs/doc-export/blocks-mapping/defaultBlocksDocxJustify.tsxsrc/frontend/apps/impress/src/features/docs/doc-export/blocks-mapping/imageDocx.tsxsrc/frontend/apps/impress/src/features/docs/doc-export/blocks-mapping/index.tssrc/frontend/apps/impress/src/features/docs/doc-export/mappingDocx.tsxsrc/frontend/apps/impress/src/features/docs/doc-export/utils.ts
| shading: | ||
| props.backgroundColor === 'default' || !props.backgroundColor | ||
| ? undefined | ||
| : { | ||
| type: ShadingType.CLEAR, | ||
| fill: (() => { | ||
| const color = colors[props.backgroundColor]?.background; | ||
| if (!color) { | ||
| return undefined; | ||
| } | ||
| return color.slice(1); | ||
| })(), | ||
| }, | ||
| run: | ||
| props.textColor === 'default' || !props.textColor | ||
| ? undefined | ||
| : { | ||
| color: (() => { | ||
| const color = colors[props.textColor]?.text; | ||
| if (!color) { | ||
| return undefined; | ||
| } | ||
| return color.slice(1); | ||
| })(), | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
In the docxnpm library, areParagraphoption fields likeshading.fillandrun.colorallowed to beundefined, or should the entire shading/run object be omitted when color is unresolved?
💡 Result:
Yes—within docx, option fields such as shading.fill and run.color are defined as optional, and passing them as undefined (i.e., leaving them undefined) is supported; docx omits the corresponding XML attribute when the value is undefined. Therefore, when a color is unresolved, you can set the individual option fields to undefined rather than needing to omit the entire shading/run object. In docx’s shading implementation, the IShadingAttributesProperties type defines fill/color/type as optional fields, and createShading maps undefined to “no attribute” by setting the XML attribute value to undefined when fill/color are undefined [1]. This means the library naturally supports “unresolved” colors by omitting only those attributes, not the entire parent object. Similarly for run-level color: docx’s public type definitions for run properties expose color?: string (optional) [2], meaning you should provide it only when you have a concrete value. If you omit/leave it undefined, the serialization layer can exclude it (and RunProperties.prepForXml is designed to exclude empty components from XML serialization) [3]. Practical guidance: - If you have a Paragraph/Run options object and you don’t know the color yet, omit the specific fields (or leave them as undefined). - Only omit the entire shading or run object if the whole component would be empty/unneeded for your desired formatting. Answering your exact question: you do not need to omit the entire shading/run object when shading.fill or run.color is unresolved; docx is designed to allow those individual fields to be undefined and will omit the underlying XML attributes [1][2].
Citations:
- 1: https://github.com/dolanmiu/docx/blob/9439c73871e3ac9af5a5889978b7fbea9f3b6a2f/src/file/shading/shading.ts
- 2: https://docx.js.org/api/types/IRunStylePropertiesOptions.html
- 3: https://docx.js.org/api/classes/RunProperties.html
DOCX accepts undefined style fields; omitting the whole shading/run object isn’t required
docx’s types/serialization support optional style attributes like shading.fill and run.color; leaving those fields undefined simply omits the corresponding XML attributes. You can still choose to omit the entire shading/run block when the lookup fails to reduce emitted XML, but the current undefined fill/color values shouldn’t cause invalid output.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/frontend/apps/impress/src/features/docs/doc-export/blocks-mapping/blockNoteDocxBlockProps.ts`
around lines 19 - 43, The current code conditionally omits the entire
shading/run objects when backgroundColor/textColor are 'default' or missing, but
docx can accept undefined subfields; simplify by always returning the shading
object (with type: ShadingType.CLEAR) and the run object while setting
shading.fill to colors[props.backgroundColor]?.background?.slice(1) (or
undefined) and run.color to colors[props.textColor]?.text?.slice(1) (or
undefined), removing the outer ternaries that produce undefined for the whole
shading/run so invalid XML won’t be produced and lookups that miss a color
simply leave the subfield undefined; update the code around the shading and run
expressions (references: shading, ShadingType.CLEAR, run, colors,
props.backgroundColor, props.textColor) accordingly.
1 participant
Purpose
Expose justified text alignment in the editor so formal documents match common expectations (e.g. justified body text). Docs is gaining adoption in French administration contexts where official or formal content often expects full justification alongside left / center / right.
We are aware justified text is not always ideal for accessibility (word spacing) (#395). We believe usage will stay relatively uncommon for everyday notes; the control remains optional (per block, not a default). For the workflows that need it, having justify available is nonetheless important.
Proposal
TextAlignButtonforjustify, alongside left / center / right (minimal surface: reuses@blocknote/react+ existing i18n).justify; default toolbar omitted it—we only surface it.@blocknote/xl-docx-exporterhistorically maps justify to OOXMLdistribute, which is not the same as standard justified paragraphs (both). We prefer upstream fix or typed overrides long-term over patching minifieddist, which is brittle on exporter upgrades—scope any DOCx alignment tweak in this PR or as a follow-up as maintainers prefer.External contributions
Thank you for your contribution! 🎉
Please ensure the following items are checked before submitting your pull request:
General requirements
Skip the checkbox below 👇 if you're fixing an issue or adding documentation
CI requirements
git commit --signoff(DCO compliance)git commit -S)<gitmoji>(type) title description## [Unreleased]section (if noticeable change)AI requirements
Skip the checkboxes below 👇 If you didn't use AI for your contribution