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

Added touchmove support to slider component #5856

Merged
merged 3 commits into from
Aug 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions draftlogs/5856_add.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Add touch support to `slider` component [[#5856](https://github.com/plotly/plotly.js/pull/5856)],
with thanks to @keul for the contribution!
23 changes: 17 additions & 6 deletions src/components/sliders/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ function attachGripEvents(item, gd, sliderGroup) {
return sliderGroup.data()[0];
}

item.on('mousedown', function() {
function mouseDownHandler() {
var sliderOpts = getSliderOpts();
gd.emit('plotly_sliderstart', {slider: sliderOpts});

Expand All @@ -475,25 +475,36 @@ function attachGripEvents(item, gd, sliderGroup) {
handleInput(gd, sliderGroup, sliderOpts, normalizedPosition, true);
sliderOpts._dragging = true;

$gd.on('mousemove', function() {
function mouseMoveHandler() {
var sliderOpts = getSliderOpts();
var normalizedPosition = positionToNormalizedValue(sliderOpts, d3.mouse(node)[0]);
handleInput(gd, sliderGroup, sliderOpts, normalizedPosition, false);
});
}

$gd.on('mousemove', mouseMoveHandler);
$gd.on('touchmove', mouseMoveHandler);

$gd.on('mouseup', function() {
function mouseUpHandler() {
var sliderOpts = getSliderOpts();
sliderOpts._dragging = false;
grip.call(Color.fill, sliderOpts.bgcolor);
$gd.on('mouseup', null);
$gd.on('mousemove', null);
$gd.on('touchend', null);
$gd.on('touchmove', null);

gd.emit('plotly_sliderend', {
slider: sliderOpts,
step: sliderOpts.steps[sliderOpts.active]
});
});
});
}

$gd.on('mouseup', mouseUpHandler);
$gd.on('touchend', mouseUpHandler);
}

item.on('mousedown', mouseDownHandler);
item.on('touchstart', mouseDownHandler);
}

function drawTicks(sliderGroup, sliderOpts) {
Expand Down
4 changes: 4 additions & 0 deletions test/jasmine/assets/touch_event.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ module.exports = function(type, x, y, opts) {
target: el,
clientX: x,
clientY: y,
screenX: x,
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 was forced to complete the signature of the event or simulating the touch event on the slider will generate a very strange behavior (slider was always moved on the start position).

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to add these? It looks the test passes without it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@archmoj not on my machine (MacOS). Just tested again removing these lines and I have the failure i fixed adding this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW: I'm able to reproduce the issue also outside Jasmine/jsdom.

issue

I'm 100% for removing this if you think it's just an issue on my Chrome… but seems really strange to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for testing.
LGTM 🏅
Over to @alexcjohnson

screenY: y,
pageX: x,
pageY: y,
radiusX: 2.5,
radiusY: 2.5,
rotationAngle: 10,
Expand Down
39 changes: 39 additions & 0 deletions test/jasmine/tests/sliders_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ var Plotly = require('@lib/index');
var Lib = require('@src/lib');
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var touchEvent = require('../assets/touch_event');

var delay = require('../assets/delay');
var assertPlotSize = require('../assets/custom_assertions').assertPlotSize;
Expand Down Expand Up @@ -507,6 +508,44 @@ describe('sliders interactions', function() {
.then(done, done.fail);
});

it('should respond to touch interactions', function(done) {
var firstGroup = gd._fullLayout._infolayer.select('.' + constants.railTouchRectClass);
var firstGrip = gd._fullLayout._infolayer.select('.' + constants.gripRectClass);
var railNode = firstGroup.node();
var gripNode = firstGrip.node();
var touchRect = railNode.getBoundingClientRect();
var gripRect = gripNode.getBoundingClientRect();

var originalFill = gripNode.style.fill;

expect(mockCopy.layout.sliders[0].active).toEqual(2);

// Dispatch start of touch where the grip control is
touchEvent('touchstart', gripRect.left + 5, gripRect.top + 5);

expect(mockCopy.layout.sliders[0].active).toEqual(2);
var touchdownFill = gripNode.style.fill;
expect(touchdownFill).not.toEqual(originalFill);

// Drag to the right side:
touchEvent('touchmove', touchRect.left + touchRect.width - 5, gripRect.top + 5);

var touchmoveFill = gripNode.style.fill;
expect(touchmoveFill).toEqual(touchdownFill);

delay(100)()
.then(function() {
expect(mockCopy.layout.sliders[0].active).toEqual(5);

touchEvent('touchend', touchRect.left + touchRect.width - 5, gripRect.top + 5);

var touchupFill = gripNode.style.fill;
expect(touchupFill).toEqual(originalFill);
expect(mockCopy.layout.sliders[0].active).toEqual(5);
})
.then(done, done.fail);
});

it('should issue events on interaction', function(done) {
var cntStart = 0;
var cntInteraction = 0;
Expand Down