Skip to content

Add guideline between 2 points#2226

Merged
justvanrossum merged 8 commits into
fontra:mainfrom
752986:guideline-between-2-points
Aug 11, 2025
Merged

Add guideline between 2 points#2226
justvanrossum merged 8 commits into
fontra:mainfrom
752986:guideline-between-2-points

Conversation

@752986

@752986 752986 commented Jul 15, 2025

Copy link
Copy Markdown
Contributor

When 2 points are selected, a new context menu item is enabled to create a guideline between them. This resolves the more important half of #2219.

Hopefully the way I'm getting the currently selected points is reasonable; I had some trouble navigating the codebase so maybe there's a better method hiding somewhere.

Besides translations, is anything else needed in order to consider this change complete?

@google-cla

google-cla Bot commented Jul 15, 2025

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@justvanrossum justvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you! I left some comments.

Comment thread src-js/fontra-core/assets/lang/en.js
Comment thread src-js/views-editor/src/editor.js Outdated
Comment thread src-js/views-editor/src/editor.js Outdated
Comment thread src-js/views-editor/src/editor.js Outdated
@752986

752986 commented Jul 24, 2025

Copy link
Copy Markdown
Contributor Author

Alright, I've added code to place the origin between the points when possible, but testing on some real glyphs, that doesn't seem to happen extremely often. I could explore giving it some tolerance, which would pretty much guarantee a centered point, but I imagine that would add a lot of complexity.

I'll have to use this version for a bit to see, but I wonder if the unpredictability in placement will be more annoying than it not being centered.

@justvanrossum

Copy link
Copy Markdown
Member

You're probably avoiding placing it on point A because that makes the guideline hard to select? Ideally guidelines should be selectable all along the guideline, but that's a change we should probably tackle separately.

@justvanrossum

justvanrossum commented Jul 24, 2025

Copy link
Copy Markdown
Member

I implemented "click anywhere on guideline, not just the anchor point" in https://github.com/googlefonts/fontra/pull/2234. So if you can rebase on main, you can play with it in the context of your work. I think it's a good improvement.

@752986

752986 commented Jul 25, 2025

Copy link
Copy Markdown
Contributor Author

Awesome, that's a much better way to solve the problem. The one thing I notice is that even when selected, the guideline is still behind the path point, so it's hard to tell that it is selected. Perhaps the entire guideline should become darker when selected, so you can tell even when its origin is off screen.

@justvanrossum

Copy link
Copy Markdown
Member

The one thing I notice is that even when selected, the guideline is still behind the path point, so it's hard to tell that it is selected. Perhaps the entire guideline should become darker when selected, so you can tell even when its origin is off screen.

Yeah, that's true. I briefly looked into that and decided that's "for later", because that code is more complex than it should be, but if you want to give it a go, feel free.

@752986

752986 commented Jul 28, 2025

Copy link
Copy Markdown
Contributor Author

If I do work on that, it'll be in a separate pull request.

@justvanrossum justvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

@justvanrossum justvanrossum merged commit d02db1d into fontra:main Aug 11, 2025
5 checks passed
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.

2 participants