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

Highlight component when dragging on X and Y not taking Y margin into account #1243

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

Conversation

s-heap
Copy link

@s-heap s-heap commented Sep 4, 2019

Added "- marginTop" in the _convertAreaToCoordinates function when enableX and enableY are set to account for the vertical margin and prevent inaccuracies in y selection.

A pull request to fix the issue mentioned in #1238 regarding the highlight component inaccuracies.

@CLAassistant
Copy link

CLAassistant commented Sep 4, 2019

CLA assistant check
All committers have signed the CLA.

…ableX and enableY are set to account for the vertical margin and prevent inaccuracies in y selection.
@s-heap s-heap force-pushed the bug/highlight-y-margin-patch branch from 8396b3f to a422525 Compare September 4, 2019 09:45
@mcnuttandrew
Copy link
Contributor

mcnuttandrew commented Sep 5, 2019

I think this was handled in another PR, try rebasing and see where you get. (Also thanks for contributing it's big appreciated)

@s-heap
Copy link
Author

s-heap commented Sep 5, 2019

I've had a look but haven't been able to find a relevant pull request for this issue.
If you have any other information on how to find it that would be useful.

@mcnuttandrew
Copy link
Contributor

i 100% misremembered my bad, long week. the other pr resolved a similar error in a different place in the code

@s-heap
Copy link
Author

s-heap commented Sep 9, 2019

That's fair I understand. I've come across a few errors and issues that I think are linked to the system not taking margins properly into account but this is the only one I've found and nailed down as of yet.

@Burbenog
Copy link

I there any chance this will be merged any time soon?

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.

4 participants