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

[background image] Front-End updates (selectable, editable and transformable) #1783

Merged
merged 32 commits into from
Nov 15, 2024

Conversation

ollimeier
Copy link
Collaborator

@ollimeier ollimeier commented Nov 13, 2024

This fixes #1782

@ollimeier
Copy link
Collaborator Author

@justvanrossum backgroundImageSelectionAtPoint currently returns a set, to make it equal to the other selection functions.
But for now, only one background image can be selected, so it could be a boolean, right?
But this would be inconsistent with the other selection functions. What do you think?
Do we limit the background image to one image only or do we eventually allow multiple images in future?

@justvanrossum
Copy link
Collaborator

Please keep it a set, both for consistency with the other methods, as well as for possible future expansion to multiple images.

@justvanrossum
Copy link
Collaborator

Will this PR implement all of #1782, or just part of it?

@ollimeier
Copy link
Collaborator Author

Will this PR implement all of #1782, or just part of it?

What do you prefer? Both is still possible: as separate PRs or merge them into one and then have all of #1782 in one PR.

@ollimeier ollimeier force-pushed the issue-1782-make-the-background-image branch from 0aac674 to fbdaaed Compare November 13, 2024 08:16
@justvanrossum
Copy link
Collaborator

Please use a better title than "Make the background image".

@justvanrossum
Copy link
Collaborator

justvanrossum commented Nov 13, 2024

And use [background image] as a prefix for all PR and issue titles related to the bg image tasks.

@justvanrossum
Copy link
Collaborator

Since it does not make sense to merge a "selectable" PR separarate from "editable" and "transformable", these three should for sure be one PR. Depending on the complecity "deletable" may be better as a followup.

@justvanrossum
Copy link
Collaborator

So, when I say "separate PRs", I mean "sequential PRs", not "nested PRs", ie. please please please no PRs on PRs, that just brings utter chaos.

@ollimeier ollimeier force-pushed the issue-1782-make-the-background-image branch from 471e482 to 493eb87 Compare November 13, 2024 09:18
@ollimeier ollimeier changed the title Make the background image [background image] Front-End updates (selectable, editable and transformable) Nov 13, 2024
@ollimeier
Copy link
Collaborator Author

Please use a better title than "Make the background image".

I changed the title to: [background image] Front-End updates (selectable, editable and transformable)

@ollimeier ollimeier force-pushed the issue-1782-make-the-background-image branch from 0ff56f2 to cb086a4 Compare November 14, 2024 00:19
src/fontra/views/editor/edit-behavior.js Outdated Show resolved Hide resolved
src/fontra/client/lang/ja.js Show resolved Hide resolved
src/fontra/client/core/glyph-controller.js Outdated Show resolved Hide resolved
src/fontra/client/core/glyph-controller.js Outdated Show resolved Hide resolved
src/fontra/views/editor/edit-behavior.js Outdated Show resolved Hide resolved
src/fontra/views/editor/scene-model.js Outdated Show resolved Hide resolved
src/fontra/client/core/glyph-controller.js Outdated Show resolved Hide resolved
@justvanrossum justvanrossum merged commit 2d3797f into main Nov 15, 2024
5 checks passed
@justvanrossum justvanrossum deleted the issue-1782-make-the-background-image branch November 15, 2024 09:56
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.

[background image] Make the background image selectable, editable, etc.
2 participants