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

Symbol Editor: Cannot remove coordinate pairs #2090

Open
mlerjen opened this issue Sep 14, 2022 · 9 comments · May be fixed by #2093
Open

Symbol Editor: Cannot remove coordinate pairs #2090

mlerjen opened this issue Sep 14, 2022 · 9 comments · May be fixed by #2093

Comments

@mlerjen
Copy link
Contributor

mlerjen commented Sep 14, 2022

Steps to reproduce

  1. Defining a point symbol with a line element
  2. Adding coordinate pairs
  3. Want to remove an unnecessary coordinate pair

Actual behaviour

At certain coordinate pairs the minus-button is grayed out, for others it is not.
image

Expected behaviour

Minus button should be always on IMO.

Configuration

Mapper Version: Master v.20220316.2
Operating System: Windows7

@dl3sdo
Copy link
Member

dl3sdo commented Sep 14, 2022

I think that this inability to remove coordinate pairs is related to the Curve property above.
Try to uncheck the 'Curve start' checkmark, remove the coordinate pair and then make it a curve again.

@dl3sdo
Copy link
Member

dl3sdo commented Sep 14, 2022

However, this dialog faces some issues.
When playing around you can end up in a situation with a curve but without any checkbox at all, e.g.:
CurveWithoutCheckbox
Minimum steps to reproduce:

  1. Define a point symbol with a line element
  2. Adding 4 points and thus 3 line segments by clicking inside the right panel => the 'Curve start' checkbox appears at the topmost row
  3. Check the checkbox => line gets converted into a curve
  4. Activate topmost row and add new row by pressing the '+' button
    => checkbox disappears but line remains as curve

@mlerjen
Copy link
Contributor Author

mlerjen commented Sep 14, 2022

  • Testing shows it is correct, that coordinates involved in a curve can not be deleted. But in my case the blocking could not be released anymore, even after unchecking all available curve property check-boxes.
  • Also in both my and your example the curve check boxes are not shown at places they should (at the third line after the last checked box).
  • I suspect, that a checked box is not shown and therefore can not be unchecked nor can the point be deleted.

dl3sdo added a commit to dl3sdo/mapper that referenced this issue Sep 18, 2022
When adding a path point by clicking on the '+' button either a default
point is appended to the path or an existing path point is duplicated.
However, when duplicating an existing point, also the CurveStart
property was duplicated which lead to two successive path points with
a set CurveStart property although the next two path points following
a CurveStart path point define the course of the curve and thus can't
be a CurveStart path point itself.

Resolves OpenOrienteering#2090
@dl3sdo
Copy link
Member

dl3sdo commented Sep 18, 2022

@mlerjen: thank you for reporting this issue.
The reason for the issue was that it was possible to create erroneous line parts that contained two successive line segments which claimed to start a curve. The point symbol editor however showed checkboxes based on the assumption of correct line parts.
Although it seems as if Mapper silently ignored the second CurveStart property, it's good to have found and removed this hidden error.

@dl3sdo
Copy link
Member

dl3sdo commented Sep 18, 2022

Current workaround:
Before selecting a row that is marked as 'Curve start' and where the '+' button shall duplicate this row, first deselect 'Curve start', duplicate row and then select 'Curve start' again.

dl3sdo added a commit to dl3sdo/mapper that referenced this issue Dec 10, 2024
When adding a path point by clicking on the '+' button either a default
point is appended to the path or an existing path point is duplicated.
However, when duplicating an existing point, also the CurveStart
property was duplicated which lead to two successive path points with
a set CurveStart property although the next two path points following
a CurveStart path point define the course of the curve and thus can't
be a CurveStart path point itself.

Closes OpenOrienteeringGH-2090 (Symbol Editor: Cannot remove coordinate pairs).
dl3sdo added a commit to dl3sdo/mapper that referenced this issue Dec 27, 2024
When adding a path point by clicking on the '+' button either a default
point is appended to the path or an existing path point is duplicated.
However, when duplicating an existing point, also the CurveStart
property was duplicated which lead to two successive path points with
the CurveStart property being set although the next two path points
following a CurveStart path point define the course of the curve and
thus can't be a CurveStart path point itself.

Closes OpenOrienteeringGH-2090 (Symbol Editor: Cannot remove coordinate pairs).
@dl3sdo
Copy link
Member

dl3sdo commented Dec 28, 2024

@mlerjen: you may try the current build containing the fix (#2093).

@mlerjen
Copy link
Contributor Author

mlerjen commented Jan 8, 2025

I tested this with v20250105.2
Now I can somehow delete every point I like, but the behavior is still a bit inconsistent as checking/unchecking the curve start box does not update the "deletability" of the points involved in this curve segment/former curve segment.

Steps to reproduce:

  • In Symbol settings editor select a point involved in a curve: The point can not be deleted
  • Uncheck the Curve start checkbox: The point still can not be deleted
  • Deselect and re-select the point you want to delete: The point now can be deleted

and the inverse case

  • Select a point not involved into a curve: It can be deleted
  • Check a curve start box to involve this point into a curve: It still can be deleted
  • De-select and re-select the point involved in a curve: I no longer can be deleted.

I think to be correct, we should re-evaluate the "deletability" of a selected point on every change in the curve start column. Right ??

@dl3sdo
Copy link
Member

dl3sdo commented Jan 8, 2025

v20250105.2 does not yet contain the fix as it's still a PR. The previous build is no longer available.
I'll add a commit that will trigger a new build.

@dl3sdo
Copy link
Member

dl3sdo commented Jan 8, 2025

Please test 20250108.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants