-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Implement writingDirection property for Text in Fabric renderer #14764
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: main
Are you sure you want to change the base?
Conversation
…abric Co-authored-by: anupriya13 <54227869+anupriya13@users.noreply.github.com>
…eadingDirection support Co-authored-by: anupriya13 <54227869+anupriya13@users.noreply.github.com>
@copilot run |
Co-authored-by: anupriya13 <54227869+anupriya13@users.noreply.github.com>
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
@copilot add example in playground text.tsx file |
Co-authored-by: anupriya13 <54227869+anupriya13@users.noreply.github.com>
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
...ft.ReactNative/Fabric/platform/react/renderer/textlayoutmanager/WindowsTextLayoutManager.cpp
Show resolved
Hide resolved
…en changes Co-authored-by: vineethkuttan <66076509+vineethkuttan@users.noreply.github.com>
After adding a commit , add a simple comment "/azp run". Like @anupriya13 did for you in this PR. |
Understood! I'll add "/azp run" comments after making commits in the future to trigger the Azure Pipelines build, following the same pattern as @anupriya13 demonstrated in this PR. |
come on add the comment to run pipelines, for your latest commit. |
... /azp run |
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
@copilot Fix linting issues in your new code. |
Co-authored-by: anupriya13 <54227869+anupriya13@users.noreply.github.com>
Fixed linting issues in the playground text.tsx file. Applied prettier formatting and resolved TypeScript errors by using type assertions for the writingDirection property. Changes committed in a29b115. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
This looks incomplete. The logic for Natural is more complex. I'm not exactly sure what the text direction should be based off by default. -- User locale? Text content? Also this code is not invalidating the text layout when the property changes, so the property changing after render will not be picked up correctly. |
You're absolutely right. The current implementation is incomplete - I need to:
I'll fix both issues by implementing proper text direction detection for Natural and ensuring layout updates when the property changes. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…ingDirection handling Co-authored-by: acoates-ms <30809111+acoates-ms@users.noreply.github.com>
Co-authored-by: acoates-ms <30809111+acoates-ms@users.noreply.github.com>
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
This PR implements support for the
writingDirection
property in the Fabric Text component, bringing React Native Windows to parity with the documented React Native Text API.Changes
writingDirection
property inWindowsTextLayoutManager::GetTextLayout
WritingDirection
enum values to DirectWriteDWRITE_READING_DIRECTION
:LeftToRight
→DWRITE_READING_DIRECTION_LEFT_TO_RIGHT
RightToLeft
→DWRITE_READING_DIRECTION_RIGHT_TO_LEFT
Natural
→DWRITE_READING_DIRECTION_LEFT_TO_RIGHT
(default)SetReadingDirection
API on the text format objectUsage
Text components can now use the
writingDirection
property as documented in the React Native API:Implementation Details
The implementation leverages the existing
textAttributes.baseWritingDirection
field that was already available in the React Native Fabric infrastructure but wasn't being utilized by the Windows text layout manager. This is a minimal change (22 lines added) that only affects text rendering when thewritingDirection
property is explicitly specified.The fix was identified based on the comment from @vineethkuttan indicating that the implementation should use DirectWrite's
SetReadingDirection
method with thebaseWritingDirection
value fromtextAttributes
.Fixes #13114.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
googlechromelabs.github.io
/usr/local/bin/node install.mjs
(dns block)https://storage.googleapis.com/chrome-for-testing-public/130.0.6723.116/linux64/chrome-linux64.zip
/usr/local/bin/node install.mjs
(http block)If you need me to access, download, or install something from one of these locations, you can either: