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 coordinate hints in the GraphTool for restricted sine wave points. #1185

Merged
merged 3 commits into from
Feb 4, 2025

Conversation

drgrice1
Copy link
Member

After construction of a sine wave is completed, if the point that determines amplitude is selected and moved with the mouse, then the x-coordinate of the point can't change. However, the x-coordinate of the coordinate hints in the lower right corner of the board does change if the mouse is moved to the left and right while dragging that point, and should not be.

The same thing happens with the point that determines period on relative to the vertical axis. The y-coordinate of that point can't change, but the y-coordinate of the coordinate hints do change if the mouse is moved up or down.

This pull request fixes the issue by overriding the default graph object updateTextCoords method for the sine wave graph object. A flag set on the point when it is dragged, and the updateTextCoords override checks for the flag and sets the coordinate hints with the coordinates of the point instead of using the mouse cursor coordinates which might be incorrect.

This is similar to the issue that @Alex-Jordan noticed in #1157, although that was during the construction phase of the sine wave.

To test you can use the attached problem: GraphSineWavePi.pg.txt

In that problem, graph a sine wave by plotting all three points. Then click on either the amplitude or period point (the second or third point graphed during the construction phase), and try to drag the point in the direction it can't move. With the develop branch you will see the coordinate change in the direction the point can't go. With this pull request that coordinate stays fixed.

@drgrice1 drgrice1 marked this pull request as draft January 29, 2025 04:45
@drgrice1
Copy link
Member Author

I have converted this to a draft pull request because I found that this issue is not new with the sine wave tool. This affects all graph objects that have restricted point drags. I will fix that issue more generally in this pull request.

@drgrice1 drgrice1 force-pushed the graphtool-sinewave-coords-fix branch from 9785d64 to 6f67f37 Compare January 29, 2025 16:08
@drgrice1 drgrice1 marked this pull request as ready for review January 29, 2025 16:14
@drgrice1
Copy link
Member Author

This is ready now.

Basically the full issue is that any time that a point's drag is restricted the coordinate hints can be wrong. This happens if the drag is in a range that the point can't be moved to and can happen for both mouse or keyboard drags.

For an extreme example where this is easy to see, with the 4-point cubic tool graph the points (-1, 0), (0, 0), (1, 0), and (2, 0). Then click on the point at (2, 0) and slowly drag it to the left. Without this pull request you will see the coordinate hints show (1, 0), then (0, 0), etc., even though that is not where the point being dragged is.

Or graph the same points for a cubic as above, then click on the fourth point graphed (or use Tab to keyboard navigate to that point) after completion of graphing the cubic. Then use the left arrow on the keyboard to move the point left. It will move to the left of the other three points, but the coordinates shown will be incorrect without this pull request. If you then use the right arrow to move it back to the other side of the other three points the coordinates will change, but will still be incorrect without this pull request.

@drgrice1
Copy link
Member Author

Also, these incorrect coordinates can happen during the construction phase as well (again without this pull request).

@drgrice1
Copy link
Member Author

By the way, here is a general problem with all of the tools for testing:
Demo.pg.txt

Copy link
Contributor

@Alex-Jordan Alex-Jordan left a comment

Choose a reason for hiding this comment

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

Tested and works as described.

After construction of a sine wave is completed, if the point that
determines amplitude is selected and moved with the mouse, then the
x-coordinate of the point can't change.  However, the x-coordinate of
the coordinate hints in the lower right corner of the board does change
if the mouse is moved to the left and right while dragging that point,
and should not be.

The same thing happens with the point that determines period on relative
to the vertical axis.  The y-coordinate of that point can't change, but
the y-coordinate of the coordinate hints do change if the mouse is moved
up or down.

This pull request fixes the issue by overriding the default graph object
`updateTextCoords` method for the sine wave graph object.  A flag set on
the point when it is dragged, and the `updateTextCoords` override
checks for the flag and sets the coordinate hints with the coordinates
of the point instead of using the mouse cursor coordinates which might
be incorrect.

This is similar to the issue that @Alex-Jordan noticed in openwebwork#1157,
although that was during the construction phase of the sine wave.
Any time that a point's drag is restricted the coordinate hints can be
wrong. This happens if the drag is in a range that the point can't be
moved to and can happen for both mouse or keyboard drags.

For an extreme example where this is easy to see, with the 4-point cubic
tool graph the points (-1, 0), (0, 0), (1, 0), and (2, 0).  Then click
on the point at (2, 0) and slowly drag it to the left. Without this pull
request you will see the coordinate hints show (1, 0), then (0, 0),
etc., even though that is not where the point being dragged is.
methods in the down/up handlers rather than all of the code replication.

Also use these for the first points that are created for some of the
tools like the line tool that wasn't yet getting the needed handler.
@drgrice1 drgrice1 force-pushed the graphtool-sinewave-coords-fix branch from 46e2ac4 to f4a716e Compare February 1, 2025 23:37
Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

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

Looks like it fixes the issue.

@pstaabp pstaabp merged commit c5590e3 into openwebwork:develop Feb 4, 2025
3 checks passed
@drgrice1 drgrice1 deleted the graphtool-sinewave-coords-fix branch February 4, 2025 20:18
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.

3 participants