Skip to content
This repository was archived by the owner on Dec 19, 2024. It is now read-only.

Use effective direction when attached to mirror slider X #191

Open
danbeam wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use effective direction when attached to mirror slider X #191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions paper-slider.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
outline: none;
}

:host-context([dir="rtl"]) #sliderContainer {
:host([_is-r-t-l]) #sliderContainer {
-webkit-transform: scaleX(-1);
transform: scaleX(-1);
}
Expand Down Expand Up @@ -462,6 +462,11 @@
return [];
}
},

_isRTL: {
type: Boolean,
reflectToAttribute: true,
},
},

observers: [
Expand All @@ -483,6 +488,10 @@
'up pageup end': '_incrementKey'
},

attached: function() {
this._isRTL = window.getComputedStyle(this)['direction'] === 'rtl';
Copy link
Contributor

@keanulee keanulee May 18, 2017

Choose a reason for hiding this comment

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

The getComputedStyle() in attached is costly, and could degrade performance significantly especially if there are multiple instances of this element. Don't know of any alternatives tho - thoughts @frankiefu?

Copy link
Contributor

@frankiefu frankiefu May 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, as @keanulee points out getComputedStyle will force layout and so not really ideal here. There is :dir() CSS pseudo-class which will solve this problem but it's not implemented in Chrome yet. So without :dir or host-context one workaround is to manually check for dir attribute on the parent and the body. @sorvell did a quick prototype on this: https://glitch.com/edit/#!/fern-scarf

Copy link
Contributor Author

@danbeam danbeam May 19, 2017

Choose a reason for hiding this comment

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

@frankiefu yep, I know about :dir() and how it doesn't work in Chrome yet.

Did you read the bug? #190 it talks about how just checking [dir] is not good enough.

Copy link
Contributor

@frankiefu frankiefu May 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Unfortunately without better platform support this is the limitation we have to endure without sacrificing performance. It seems to me the biggest issue is currently there is no way to force the direction of the slider to be LTR when the page's has a dir="rtl". I'd suggest we just make it to support if someone wants to override the dir it has to be set on paper-slider <paper-slider dir="ltr">.

},

/**
* Increases value by `step` but not above `max`.
* @method increment
Expand Down Expand Up @@ -689,13 +698,6 @@
});
},

get _isRTL() {
if (this.__isRTL === undefined) {
this.__isRTL = window.getComputedStyle(this)['direction'] === 'rtl';
}
return this.__isRTL;
},

_leftKey: function(event) {
if (this._isRTL)
this._incrementKey(event);
Expand Down
22 changes: 22 additions & 0 deletions test/rtl.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@
</template>
</test-fixture>

<test-fixture id="cssDirectionRtl">
<template>
<paper-slider value="50" style="direction: rtl;"></paper-slider>
</template>
</test-fixture>

<test-fixture id="nestedDirAttributes">
<template>
<div dir="rtl">
<paper-slider value="50" dir="ltr"></paper-slider>
</div>
</template>
</test-fixture>

<script>
suite('<paper-slider>', function() {
var slider;
Expand Down Expand Up @@ -99,6 +113,14 @@
assert.equal(oldValue - slider.step, slider.value);
assert.isTrue(down.defaultPrevented);
});

test('css direction works', function() {
assert.isTrue(fixture('cssDirectionRtl')._isRTL);
});

test('nested [dir] attributes work', function() {
assert.isFalse(fixture('nestedDirAttributes').firstElementChild._isRTL);
});
});

</script>
Expand Down