Skip to content

Placeholder for selected glyph - #2206

Merged
justvanrossum merged 19 commits into
mainfrom
canvas_placeholder_for_selected_glyph
Jul 14, 2025
Merged

Placeholder for selected glyph#2206
justvanrossum merged 19 commits into
mainfrom
canvas_placeholder_for_selected_glyph

Conversation

@GaetanBaehr

@GaetanBaehr GaetanBaehr commented Jul 3, 2025

Copy link
Copy Markdown
Collaborator

This fixes #2198: replacing in the canvas all the "/?" from the text entry by the selected glyph.

@GaetanBaehr
GaetanBaehr requested a review from justvanrossum July 3, 2025 12:35
@GaetanBaehr

Copy link
Copy Markdown
Collaborator Author

@justvanrossum I just pushed an update that remove the this.sceneSettings.selectedGlyph = null; in the activation of the kerningTool to keep the selected glyph in the /? and allows kern it with context

@justvanrossum

justvanrossum commented Jul 3, 2025

Copy link
Copy Markdown
Member

I just pushed an update that remove the this.sceneSettings.selectedGlyph = null; in the activation of the kerningTool to keep the selected glyph in the /? and allows kern it with context

Hm, I'm not sure we can just do that. There was a reason there can't be a selected glyph during editing kerning. This definitely needs more thinking.

There is another problem: if you use the "next/previous glyph" context menu, or use the related chars/glyph panel to insert a glyph, or use the glyph search panel to switch the selected glyph, the /? is replaced by the actual glyph. This can be solved, but will need some special handling in textFromGlyphLines.

@GaetanBaehr

Copy link
Copy Markdown
Collaborator Author

@justvanrossum I pushed a fix for the "next/previous glyph", by adding a placeholder key in the glyphNames list

Comment thread src-js/fontra-core/src/glyph-lines.js Outdated
@GaetanBaehr

Copy link
Copy Markdown
Collaborator Author

Hm, I'm not sure we can just do that. There was a reason there can't be a selected glyph during editing kerning. This definitely needs more thinking.

what was the reason ?

@justvanrossum

justvanrossum commented Jul 3, 2025

Copy link
Copy Markdown
Member

The kern tool currently does not allow selecting a glyph. It has to do with undo: normally, a selected glyph can be edited (via the info panel), but kerning has its own undo stack. So there's a lot of potential for confusion. So maybe the kern tool indeed needs to be able to select a glyph (in addition to selecting kern pairs), but then perhaps we shouldn't allow it to be edited. Or that (glyph) undo does not work while the kern tool is active.

Like I said, it needs more thinking. I don't know the best solution right now, just that as it stands it cannot work.

@GaetanBaehr

Copy link
Copy Markdown
Collaborator Author

Can't we add a key like previousSelectedGlyph and when no selected glyph the placeholder use the previousSelectedGlyph ?

@justvanrossum

Copy link
Copy Markdown
Member

Can't we add a key like previousSelectedGlyph and when no selected glyph the placeholder use the previousSelectedGlyph ?

That's worth trying!

@GaetanBaehr

Copy link
Copy Markdown
Collaborator Author

Can't we add a key like previousSelectedGlyph and when no selected glyph the placeholder use the previousSelectedGlyph ?

That's worth trying!

I just pushed it, it looks to work well

@justvanrossum

Copy link
Copy Markdown
Member

I just pushed it, it looks to work well

It's something, but it is not quite pretty: it disappears when reloading, ie. it doesn't get stored in the URL.

Like I said, it really needs more thinking, and I really need some time for that.

@justvanrossum

Copy link
Copy Markdown
Member

Like, the whole /? concept works well if the editor view is separate from the spacing/kerning view (like in RoboFont), but that's simply not what we have here.

@GaetanBaehr

Copy link
Copy Markdown
Collaborator Author

Like, the whole /? concept works well if the editor view is separate from the spacing/kerning view (like in RoboFont), but that's simply not what we have here.

Why is it not working well in our context? I'm using this pull request for spacing now, and it helps a lot

@justvanrossum

Copy link
Copy Markdown
Member

Why is it not working well in our context? I'm using this pull request for spacing now, and it helps a lot

I'm not saying it doesn't help you or doesn't work as you intended, but it does not work for the edge cases I mentioned above. It has messy behavior and the concept deserves a better solution. And like I said many times now, I need more time to think through the implications and possible better solutions.

@GaetanBaehr

Copy link
Copy Markdown
Collaborator Author

Why is it not working well in our context? I'm using this pull request for spacing now, and it helps a lot

I'm not saying it doesn't help you or doesn't work as you intended, but it does not work for the edge cases I mentioned above. It has messy behavior and the concept deserves a better solution. And like I said many times now, I need more time to think through the implications and possible better solutions.

Okay, I was not trying to push you, just trying to get a better understanding of all the implications behind this feature

@justvanrossum

Copy link
Copy Markdown
Member

I think I know how I want to finish this. I'll work on it after #2213 is done.

@justvanrossum
justvanrossum merged commit 5f0b05a into main Jul 14, 2025
@justvanrossum
justvanrossum deleted the canvas_placeholder_for_selected_glyph branch July 14, 2025 20:37
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.

Placeholder in the canvas string to display the currently edited glyph in different places (/?)

2 participants