Change step grid

[samuel-stahl]

Description

This PR adds a new dropdown to the top menu, Grid, which allows the user to change the grid size. However, there are some issues I would like to address, both on my end and in general.

Addresses issue #62

Problems

1. I have not yet changed the coordinate rounding function. Could someone tells me where it is called so I can figure out how best to change it?
2. The old logic for horizontal grid generation explicitly avoided generating grid lines on yard lines unless the yard lines were removed. However, to make it easier to extend to 1-step and 4-step grids, I changed the logic to resemble the vertical lines, and therefore redundantly generates grid lines on top of yardlines. Should we strive to keep gridline generation non-redundant, or would it be easier in the long run to keep it as I have it? For reference, the old X-Offset function is present, but commented out.
3. Due to the dimensions of the field, the 4 Step grid leaves a small horizontal space at the bottom of the field. Should we keep 4 Step grid generation as is, or should we change it somehow?

Pre-PR checklist

• Ran npm run serve and:
  • Checked basic functionality
  • Checked that errors are handled
• Ran npm run lint
• Ran npm run test:unit
• Ran npm run test:e2e and ran relevant tests
• Attached reviewers to PR and pinged on Slack/email

Screenshots/GIFs

[MichaelTamaki]

Nice work! Left some suggestions for you. And here's some comments on the questions you brought up

I have not yet changed the coordinate rounding function. Could someone tells me where it is called so I can figure out how best to change it?

Are you referring to BaseTool:roundCoordinateToGrid? https://github.com/calband/calchart-redesign/blob/main/src/tools/BaseTool.ts#L19

The old logic for horizontal grid generation explicitly avoided generating grid lines on yard lines unless the yard lines were removed. However, to make it easier to extend to 1-step and 4-step grids, I changed the logic to resemble the vertical lines, and therefore redundantly generates grid lines on top of yardlines. Should we strive to keep gridline generation non-redundant, or would it be easier in the long run to keep it as I have it? For reference, the old X-Offset function is present, but commented out.

This is fine, a couple of additional lines for the grid won't hurt! Honestly it was a performance optimization that wasn't really needed. The browser can handle thousands of SVG elements.

Due to the dimensions of the field, the 4 Step grid leaves a small horizontal space at the bottom of the field. Should we keep 4 Step grid generation as is, or should we change it somehow?

(Note that this is actually an 8 step grid that you are referring to) Great catch! This is fine, it's bound to happen... We could consider doing a grid offset? Like make the grid start from a different Y position (like -4 shifts the grid up 4 steps)? Let's create an issue for that just to keep that on our backlog.

[MichaelTamaki]

Let's rename this and fourStepGridOffsetsY to something more generic

[MichaelTamaki]

You don't have to multiply by 2 here, the X/Y scale should be 1 spacing unit to 1 step. Notice in the screenshots that the 4 step grid is actually an 8 step grid (there should be a vertical line in between each line yard, since each line yard is 8 steps between each other).

[MichaelTamaki]

This works okay, but I don't think this should be it's own top navbar item. We should reserve creating a new dropdown at the top level for new categories (For example, see the top menu of any program you use, "File", "Edit", "View", "Help", etc.)

Let's consider other options... How about the bottom menu next to all the tool icons?

[adhamrait]

I wonder if there is a way to get some tiered/tree menus in Buefy. I'm thinking something like the Format -> Number selection in google sheets. That way we can put it in the view section...

[MichaelTamaki]

Once the x2 comment is resolved in GrapherField, let's update this

Suggested change

	gridSize = 2;
	gridSize = 4;