Skip to content
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

feat: add dynamic RTL switching function + readme #120

Merged

Conversation

karimlaham
Copy link
Contributor

reference to issue:
#119

@karimlaham karimlaham mentioned this pull request Feb 6, 2024
@predikament predikament added the enhancement New feature or request label Sep 13, 2024
@predikament
Copy link
Collaborator

I think this is a fair request and I can see the usability, so I will bring it up and try to get this merged.

Cheers for creating the suggestion and PR, @karimlaham.

@Braggiouy
Copy link
Contributor

Braggiouy commented Sep 27, 2024

I have tested this modification, and it functions as expected. The writing direction can be dynamically updated by invoking the updateRtl method. The video below demonstrates this change, featuring a simple call-to-action (CTA) that triggers the updateRtl method.

video.mov

When the document is set to RTL, but the spatial navigation's direction is not updated accordingly, the focus defaults to the leftmost element instead of following the proper RTL flow.

After invoking the updateRtl method, the focus behaves correctly, adhering to the expected RTL navigation pattern, without needing to re-initialize the SpatialNavigationService.

Looks good to me. Thanks, @karimlaham, for the suggestion and PR.

cc: @predikament

@predikament predikament merged commit fd64920 into NoriginMedia:main Oct 1, 2024
@predikament
Copy link
Collaborator

Changes merged - Cheers for the contribution @karimlaham.

We'll update #119 once the new version containing this change is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants