feat: support automatic bidi direction in editor content#95
feat: support automatic bidi direction in editor content#95n00ki wants to merge 2 commits intoerictli:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIntroduces an "auto" text direction option, treats "auto" as the default, decouples text direction from layout width handling, passes textDirection into editor initialization, updates types and Tauri settings to support the new enum, and removes the CSS direction variable from the ProseMirror container. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant Settings as "Settings UI"
participant Theme as "ThemeContext"
participant Editor as "Editor"
participant Tauri as "Tauri (backend)"
User->>Settings: select text direction ("auto"/"ltr"/"rtl")
Settings->>Theme: update settings (textDirection)
Theme->>Tauri: persist settings (text_direction as enum)
Theme->>Editor: provide textDirection on init / update
Editor-->>Theme: confirm applied direction
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/types/note.ts (1)
21-21: Keep the Rust settings schema aligned withTextDirection.Line 21 tightens the frontend contract, but the persisted backend field is still
Option<String>insrc-tauri/src/lib.rsLines 100-101. That means unsupported values can still be stored and only get filtered later inThemeContext. Please switch the Rust side to an enum or validated wrapper so the settings contract is enforced end-to-end.As per coding guidelines, "Maintain type safety throughout by using TypeScript types in both frontend (src/types/) and backend (Rust structs), ensuring serialization compatibility."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/note.ts` at line 21, The frontend tightened TextDirection to "auto" | "ltr" | "rtl", but the backend still stores that setting as Option<String>, allowing invalid values; change the Rust side to use a strongly-typed enum (e.g., enum TextDirection { Auto, Ltr, Rtl }) instead of Option<String> in the settings struct (the field currently used in lib.rs), derive serde Serialize/Deserialize with the same string naming (rename_all or explicit renames) and implement a Default or Option<TextDirection> as required to preserve optionality; update any conversions/DB/serialization code that reads/writes the setting to use this enum so the contract is enforced end-to-end and serialization remains compatible with the frontend.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/types/note.ts`:
- Line 21: The frontend tightened TextDirection to "auto" | "ltr" | "rtl", but
the backend still stores that setting as Option<String>, allowing invalid
values; change the Rust side to use a strongly-typed enum (e.g., enum
TextDirection { Auto, Ltr, Rtl }) instead of Option<String> in the settings
struct (the field currently used in lib.rs), derive serde Serialize/Deserialize
with the same string naming (rename_all or explicit renames) and implement a
Default or Option<TextDirection> as required to preserve optionality; update any
conversions/DB/serialization code that reads/writes the setting to use this enum
so the contract is enforced end-to-end and serialization remains compatible with
the frontend.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 733b9c1b-ae3d-4f15-827f-e5aa58fa9f25
📒 Files selected for processing (5)
src/App.csssrc/components/editor/Editor.tsxsrc/components/settings/EditorSettingsSection.tsxsrc/context/ThemeContext.tsxsrc/types/note.ts
autoas a first-class text direction option for note contenttextDirectionsupport so mixed RTL/LTR content is detected correctlyWhy
This improves note authoring for multilingual content by letting the editor automatically detect text direction per content block, while still preserving explicit
LTRandRTLoverrides in settings.Details
src/types/note.tsTextDirectionto includeautosrc/context/ThemeContext.tsxautoautoconsistently--editor-directionCSS variablesrc/components/settings/EditorSettingsSection.tsxAutooption to the text direction settingAutoas the default valuesrc/components/editor/Editor.tsxtextDirectioninto Tiptap editor optionssrc/App.css.ProseMirrordirection override so it doesn't fight content-level auto directionSummary by CodeRabbit
New Features
Improvements