Skip to content

Commit 4c9036c

Browse files
authored
RangeSelector: use x/y attributes instead of transform for tracker positioning on Firefox+Android (T1296261) (DevExpress#30480)
1 parent 8afd636 commit 4c9036c

File tree

6 files changed

+152
-18
lines changed

6 files changed

+152
-18
lines changed

packages/devextreme/js/viz/range_selector/common.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { smartFormatter as _format } from '../axes/smart_formatter';
22
import { isFunction } from '../../core/utils/type';
3+
import browser from '../../core/utils/browser';
4+
import devices from '../../core/devices';
5+
36
export const HEIGHT_COMPACT_MODE = 24;
47
const POINTER_SIZE = 4;
58
const EMPTY_SLIDER_MARKER_TEXT = '. . .';
@@ -12,6 +15,7 @@ export const utils = {
1215
},
1316
animationSettings: { duration: 250 }
1417
};
18+
1519
export const consts = {
1620
emptySliderMarkerText: EMPTY_SLIDER_MARKER_TEXT,
1721
pointerSize: POINTER_SIZE
@@ -31,3 +35,5 @@ export const formatValue = function(value, formatOptions, tickIntervalsInfo, val
3135
};
3236
return String(isFunction(formatOptions.customizeText) ? formatOptions.customizeText.call(formatObject, formatObject) : formatObject.valueText);
3337
};
38+
39+
export const isFirefoxOnAndroid = () => browser.mozilla && devices.real().android;

packages/devextreme/js/viz/range_selector/slider.js

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { utils, formatValue } from './common';
1+
import { utils, formatValue, isFirefoxOnAndroid } from './common';
22
const animationSettings = utils.animationSettings;
33
import SliderMarker from './slider_marker';
44
import supportUtils from '../../__internal/core/utils/m_support';
@@ -16,7 +16,14 @@ function Slider(params, index) {
1616
that._sliderGroup = params.renderer.g().attr({ 'class': 'slider' }).append(params.root);
1717
that._line = params.renderer.path(null, 'line').append(that._sliderGroup);
1818
that._marker = new SliderMarker(params.renderer, that._sliderGroup, index === 1);
19-
that._tracker = params.renderer.rect().attr({ 'class': 'slider-tracker', fill: '#000000', opacity: 0.0001 }).css({ cursor: 'w-resize' }).append(params.trackersGroup);
19+
that._tracker = params.renderer.rect()
20+
.attr({
21+
'class': 'slider-tracker',
22+
fill: '#000000',
23+
opacity: 0.0001,
24+
})
25+
.css({ cursor: 'w-resize' })
26+
.append(params.trackersGroup);
2027
}
2128

2229
Slider.prototype = {
@@ -31,15 +38,22 @@ Slider.prototype = {
3138
const that = this;
3239
const slider = that._sliderGroup;
3340
const tracker = that._tracker;
34-
const attrs = { translateX: that._position };
41+
42+
const sliderAttrs = { translateX: that._position };
43+
let trackerAttrs = { translateX: that._position };
44+
45+
if(isFirefoxOnAndroid()) {
46+
trackerAttrs = { x: that._position - (tracker._originalWidth / 2) };
47+
}
3548

3649
that._marker.setPosition(that._position);
50+
3751
if(isAnimated) {
38-
slider.animate(attrs, animationSettings);
39-
tracker.animate(attrs, animationSettings);
52+
slider.animate(sliderAttrs, animationSettings);
53+
tracker.animate(trackerAttrs, animationSettings);
4054
} else {
41-
slider.attr(attrs);
42-
tracker.attr(attrs);
55+
slider.attr(sliderAttrs);
56+
tracker.attr(trackerAttrs);
4357
}
4458
},
4559

@@ -59,11 +73,23 @@ Slider.prototype = {
5973
that._colors = [sliderMarkerOptions.invalidRangeColor, sliderHandleOptions.color];
6074
that._sliderGroup.attr({ translateY: verticalRange[0] });
6175
that._line.attr({
62-
'stroke-width': sliderHandleOptions.width, stroke: sliderHandleOptions.color, 'stroke-opacity': sliderHandleOptions.opacity, sharp: 'h',
76+
'stroke-width': sliderHandleOptions.width,
77+
stroke: sliderHandleOptions.color,
78+
'stroke-opacity': sliderHandleOptions.opacity,
79+
sharp: 'h',
6380
points: [0, 0, 0, verticalRange[1] - verticalRange[0]]
6481
});
6582
const trackerWidth = getSliderTrackerWidth(sliderHandleOptions.width);
66-
that._tracker.attr({ x: -trackerWidth / 2, y: 0, width: trackerWidth, height: verticalRange[1] - verticalRange[0], translateY: verticalRange[0] });
83+
84+
const trackerAttrs = {
85+
x: -trackerWidth / 2,
86+
width: trackerWidth,
87+
height: verticalRange[1] - verticalRange[0],
88+
y: isFirefoxOnAndroid() ? verticalRange[0] : 0,
89+
translateY: isFirefoxOnAndroid() ? undefined : verticalRange[0]
90+
};
91+
92+
that._tracker.attr(trackerAttrs);
6793
},
6894

6995
toForeground: function() {

packages/devextreme/js/viz/range_selector/slider_marker.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { patchFontOptions } from '../core/utils';
2-
import { consts } from './common';
2+
import { consts, isFirefoxOnAndroid } from './common';
33

44
const POINTER_SIZE = consts.pointerSize;
55
const SLIDER_MARKER_UPDATE_DELAY = 75;
@@ -135,7 +135,14 @@ SliderMarker.prototype = {
135135
const offset = pointsData.offset;
136136
that._area.attr({ points: points });
137137
that._border.attr({ x: that._isLeftPointer ? points[0] - 1 : points[2], height: pointsData.isCut ? rectSize.height : rectSize.height + POINTER_SIZE });
138-
that._tracker.attr({ translateX: offset, width: rectSize.width, height: rectSize.height + POINTER_SIZE });
138+
139+
const trackerAttrs = { translateX: offset, width: rectSize.width, height: rectSize.height + POINTER_SIZE };
140+
if(isFirefoxOnAndroid()) {
141+
trackerAttrs.x = offset;
142+
trackerAttrs.translateX = undefined;
143+
}
144+
that._tracker.attr(trackerAttrs);
145+
139146
that._label.attr({ translateX: that._paddingLeftRight + offset, translateY: rectSize.height / 2 - (size.y + size.height / 2) });
140147
}
141148

packages/devextreme/js/viz/range_selector/sliders_controller.js

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { noop } from '../../core/utils/common';
2-
import { utils, consts } from './common';
2+
import { utils, consts, isFirefoxOnAndroid } from './common';
33
import Slider from './slider';
44
import { normalizeEnum as _normalizeEnum, rangesAreEqual, adjustVisualRange } from '../core/utils';
55
import { isNumeric, isDefined } from '../../core/utils/type';
@@ -169,12 +169,25 @@ SlidersController.prototype = {
169169
sliders[1].setOverlapped(areOverlapped);
170170
this._applyAreaTrackersPosition();
171171
this._applySelectedRangePosition(isAnimated);
172+
if(isFirefoxOnAndroid()) {
173+
this._areaTracker.attr({ transform: null });
174+
this._selectedAreaTracker.attr({ transform: null });
175+
this._sliders.forEach(slider => {
176+
slider._tracker.attr({ transform: null });
177+
});
178+
}
172179
},
173180

174181
_applyAreaTrackersPosition: function() {
175182
const that = this;
176-
const position1 = that._sliders[0].getPosition();
177-
const position2 = that._sliders[1].getPosition();
183+
let position1 = that._sliders[0].getPosition();
184+
let position2 = that._sliders[1].getPosition();
185+
186+
if(isFirefoxOnAndroid()) {
187+
position1 += that._sliders[0]._tracker._originalWidth / 2;
188+
position2 -= that._sliders[1]._tracker._originalWidth / 2;
189+
}
190+
178191
that._selectedAreaTracker.attr({ points: buildRectPoints(position1, that._verticalRange[0], position2, that._verticalRange[1]) }).css({
179192
cursor: Math.abs(that._params.translator.getScreenRange()[1] - that._params.translator.getScreenRange()[0] - position2 + position1) < 0.001 ? 'default' : 'pointer'
180193
});

packages/devextreme/testing/tests/DevExpress.viz.rangeSelector/common_new.tests.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ QUnit.module('RangeSelector', {
4141
});
4242

4343
// T347971
44-
QUnit.test('Empty scale is drawn with compact height when \'dataSource\' is defined and \'chart\' is not', function(assert) {
44+
QUnit.test('Empty scale is drawn with compact height when dataSource is defined and chart is not', function(assert) {
4545
this.$container.dxRangeSelector({
4646
dataSource: []
4747
});
@@ -52,7 +52,7 @@ QUnit.test('Empty scale is drawn with compact height when \'dataSource\' is defi
5252
});
5353

5454
// T347971
55-
QUnit.test('Empty scale is drawn with full height when \'dataSource\' is not defined and \'chart\' is', function(assert) {
55+
QUnit.test('Empty scale is drawn with full height when dataSource is not defined and chart is', function(assert) {
5656
this.$container.dxRangeSelector({
5757
chart: {
5858
series: {}
@@ -65,7 +65,7 @@ QUnit.test('Empty scale is drawn with full height when \'dataSource\' is not def
6565
});
6666

6767
// T347971
68-
QUnit.test('There is no unexpected incident when \'chart.series\' array is empty', function(assert) {
68+
QUnit.test('There is no unexpected incident when chart.series array is empty', function(assert) {
6969
const spy = sinon.spy();
7070
this.$container.dxRangeSelector({
7171
dataSource: [],
@@ -79,7 +79,7 @@ QUnit.test('There is no unexpected incident when \'chart.series\' array is empty
7979
});
8080

8181
// T347293
82-
QUnit.test('There is no error when \'dataSource\' is an empty array and scale is discrete datetime', function(assert) {
82+
QUnit.test('There is no error when dataSource is an empty array and scale is discrete datetime', function(assert) {
8383
const translator = this.axis.getTranslator();
8484
translator.updateBusinessRange({});
8585

packages/devextreme/testing/tests/DevExpress.viz.rangeSelector/range_selector.integration.tests.js

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import $ from 'jquery';
22
import 'viz/range_selector/range_selector';
33
import { DataSource } from 'common/data/data_source/data_source';
4+
import browser from 'core/utils/browser';
5+
import devices from '__internal/core/m_devices';
46

57
QUnit.testStart(function() {
68
const markup =
@@ -871,3 +873,83 @@ QUnit.test('Scale from dataSource. calculate linearThreshold', function(assert)
871873
assert.deepEqual(rangeSelector.getValue(), [-100, 1000]);
872874
assert.equal(rangeSelector._axis.getTranslator().getBusinessRange().linearThreshold, -4);
873875
});
876+
877+
QUnit.module('Firefox for Android adjustments (T1296261)', {
878+
createRangeSelector() {
879+
$('#container').dxRangeSelector({
880+
margin: {
881+
top: 10,
882+
},
883+
scale: {
884+
startValue: 35,
885+
endValue: 150,
886+
},
887+
value: [40, 80],
888+
behavior: {
889+
animationEnabled: false,
890+
}
891+
});
892+
},
893+
beforeEach: function() {
894+
this.createRangeSelector();
895+
},
896+
}, () => {
897+
if(!browser.mozilla || !devices.real().android) {
898+
return;
899+
}
900+
901+
QUnit.test('there should not be transform props on trackers if browser is Firefox for Android', function(assert) {
902+
const $areaTracker = $('.area-tracker');
903+
const $selectedAreaTracker = $('.selected-area-tracker');
904+
const $sliderTrackers = $('.slider-tracker');
905+
906+
assert.strictEqual($areaTracker.attr('transform'), undefined, 'area tracker does not have transform prop');
907+
assert.strictEqual($selectedAreaTracker.attr('transform'), undefined, 'selected area tracker does not have transform prop');
908+
$sliderTrackers.each((index, tracker) => {
909+
assert.strictEqual($(tracker).attr('transform'), undefined, `slider tracker ${index} does not have transform prop`);
910+
});
911+
});
912+
913+
QUnit.test('it should set x and y props instead of transform to position sliders if browser is Firefox for Android', function(assert) {
914+
const $sliderTrackers = $('.slider-tracker');
915+
916+
$sliderTrackers.each((index, tracker) => {
917+
const $tracker = $(tracker);
918+
const xCoordinate = parseInt($tracker.attr('x'));
919+
const yCoordinate = parseInt($tracker.attr('y'));
920+
921+
assert.strictEqual(xCoordinate >= 0, true, `slider tracker ${index} should have valid x position: ${xCoordinate}`);
922+
assert.strictEqual(yCoordinate > 0, true, `slider tracker ${index} should have valid y position: ${yCoordinate}`);
923+
});
924+
925+
const leftX = parseInt($sliderTrackers.eq(0).attr('x'));
926+
const rightX = parseInt($sliderTrackers.eq(1).attr('x'));
927+
assert.strictEqual(leftX < rightX, true, `left tracker x (${leftX}) is less than right slider x (${rightX})`);
928+
929+
const leftY = parseInt($sliderTrackers.eq(0).attr('y'));
930+
const rightY = parseInt($sliderTrackers.eq(1).attr('y'));
931+
assert.strictEqual(leftX < rightX, true, `left tracker y (${leftY}) is the same as the right slider y (${rightY})`);
932+
});
933+
934+
QUnit.test('selected area tracker position should be adjusted by half tracker width if browser is Firefox for Android', function(assert) {
935+
const $selectedAreaTracker = $('.selected-area-tracker');
936+
const $sliderTrackers = $('.slider-tracker');
937+
938+
const leftSliderX = parseInt($sliderTrackers.eq(0).attr('x'));
939+
const rightSliderX = parseInt($sliderTrackers.eq(1).attr('x'));
940+
941+
const trackerWidth = parseInt($sliderTrackers.eq(0).attr('width'));
942+
943+
const selectedAreaPoints = $selectedAreaTracker.attr('d');
944+
945+
const points = selectedAreaPoints.split(' ').map(p => parseFloat(p));
946+
const selectedAreaLeft = points[1];
947+
const selectedAreaRight = points[4];
948+
949+
const expectedLeft = leftSliderX + trackerWidth;
950+
const expectedRight = rightSliderX;
951+
952+
assert.roughEqual(selectedAreaLeft, expectedLeft, 1, 'Selected area left starts on the tracker end');
953+
assert.roughEqual(selectedAreaRight, expectedRight, 1, 'Selected area right ends where the right tracker starts');
954+
});
955+
});

0 commit comments

Comments
 (0)