Skip to content

Conversation

mdtusz
Copy link
Contributor

@mdtusz mdtusz commented May 24, 2016

This pr fixes #528. There was a regression when the last remaining nested <svg> tags were removed, so that when zooming with a scroll, the transforms were incorrect.

In brief:

  • add Lib.getScale/Lib.setScale similar to Lib.getTranslate/Lib.setTranslate
  • scale the plot when zooming with a scroll, based on the mouse position
  • add a mouse scroll event helper for tests - usage is mouseEvent('scroll', 100, 100, { deltaX: 0, deltaY: -500 })
  • reduce the redraw time to 50ms - this makes the scroll zoom feel much snappier in my opinion, but we may need to raise it to something a bit higher if there's issues on other machines/browsers (something closer to 100ms or 200ms)
  • added a super simple test for the scrolling - because relayout is called 50ms after the scroll is done, we need to "catch" the transformation (or alternatively we could use spies, but this seemed much simpler and will catch issues just the same)

@mdtusz mdtusz added bug something broken status: reviewable labels May 24, 2016

el.attr('transform', 'scale(0, 0); rotate(30)');
Lib.setScale(el, 30, 40);
expect(el.attr('transform')).toBe('rotate(30) scale(30, 40)');
Copy link
Contributor

Choose a reason for hiding this comment

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

nicely done.

scale = Lib.getScale(mockEl);

expect([translate.x, translate.y]).toBeCloseToArray([62.841, 99.483]);
expect([scale.x, scale.y]).toBeCloseToArray([1.221, 1.221]);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@etpinard
Copy link
Contributor

💃

@mdtusz mdtusz merged commit e369300 into master May 25, 2016
@mdtusz mdtusz deleted the zoomscroll-fix branch May 25, 2016 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

scrollZoom is incorrect on mouse scroll
2 participants