Skip to content
This repository has been archived by the owner on Sep 8, 2020. It is now read-only.

Adding RTL support #169

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

widmoser
Copy link
Contributor

@widmoser widmoser commented Dec 9, 2015

Adds right-to-left support for ui layout containers. This will automatically work as long as the ui-layout element is a child of an element with a direction set to rtl.

Fixes issue #168.

@widmoser
Copy link
Contributor Author

widmoser commented Dec 9, 2015

Tests are still missing. Every test should be also replicated in RTL mode.

@SomeKittens
Copy link
Contributor

Initial scan looks alright. Will give a full review when tests are added.

@@ -67,7 +75,6 @@ angular.module('ui.layout', [])

var debounceEvent;
function draw() {
var position = ctrl.sizeProperties.flowProperty;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this change made?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the dependency of the code on the flow property variable. It only remains when setting the css. For one, this is a refactoring step that makes the code more readable in my opinion. But more importantly it is also more robust, because the position keeps being valid even if the flow property changes (in case the layout is switched to RTL for example). Without this change the position is stored in container.left (for example), which turns into container.right when switching to RTL, a problem that I wanted to avoid.

@widmoser
Copy link
Contributor Author

@SomeKittens: I've added tests for RTL layout. From my side the PR is ready.

@asaf050
Copy link

asaf050 commented Jan 17, 2017

Hey,
Any updates for this one?

Thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants