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

fix: stop loupe being shown in incorrect position on mouseenter #113

Merged
merged 3 commits into from
Nov 2, 2018

Conversation

frederickfogerty
Copy link
Contributor

@frederickfogerty frederickfogerty commented Nov 2, 2018

Description

This library had an issue where if the mouse was quickly moved into the container and stopped, the loupe would be incorrectly positioned.

This was because the internal code was using getBoundingClientRect to calculate the width of the zoom pane, which was changing during the mouseenter animation. This animation was using CSS transforms to change the apparent width of the zoom pane. This caused the positioning logic to incorrectly position the image, and the loupe container.

This PR fixes this by using offsetWidth and offsetHeight instead, which aren't affected by transforms.

The issue:
loupebug

After solution:
loupefixed

This fixes #86.

Bug Fix

  • All existing unit tests are still passing (if applicable)
  • [N/A] Add new passing unit tests to cover the code introduced by your PR
  • [N/A] Update the readme
  • [N/A] Update or add any necessary API documentation
  • Add some steps so we can test your bug fix
  • The PR title is in the conventional commit format: e.g. fix(<area>): fixed bug #issue-number
  • Add your info to the contributors array in package.json!

Steps to Test

To test this PR:

  1. Clone this PR, and run npm install && npm run build.
  2. Update the new Drift(...) part in index.html to the snippet below to make the bug easier to reproduce.
  3. Then, run a webserver in the root of the project. I use browser-sync for this browser-sync start --server --files "dist/*".
  4. Observe that when the mouse is quickly moved over the image, and then stopped, that the loupe is initially positioned incorrectly.
  5. Checkout this PR's branch
  6. Observe that the loupe is no longer positioned incorrectly.

Code snippet to change relevant part index.html to:

		new Drift(document.querySelector('.drift-demo-trigger'), {
			paneContainer: document.querySelector('.detail'),
			hoverDelay: 200, // make this longer to make the problem more obvious
			zoomFactor: 2,
			inlinePane: true,
		});

@frederickfogerty frederickfogerty removed the request for review from jayeb November 2, 2018 19:13
@albertmejia albertmejia self-requested a review November 2, 2018 19:15
Copy link

@albertmejia albertmejia left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@frederickfogerty frederickfogerty merged commit 710dfd7 into master Nov 2, 2018
@frederickfogerty frederickfogerty deleted the fred/fix-loupe-position branch November 2, 2018 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong zoomer position on hover
2 participants