Fix per-language text layout state#11
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request isolates per-language text layout settings to prevent cross-language interference and adds support for tracking explicit layout language. The changes ensure that when switching between languages, each language retains its own font sizes and layout properties (position, offset, line height) rather than sharing a single global state.
Key changes:
- Added per-language layout settings storage via
languageSettingsobject with properties for each language - Introduced
currentLayoutLangto track the active layout language separately from text content languages - Updated text wrapping to properly handle newlines in text content
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| llm.js | Adds Gemini 3 Flash and Pro preview models to the available LLM providers |
| app.js | Implements per-language layout state isolation with new helper functions, updates UI sync logic to use language-specific settings, modifies drawing functions to apply correct layout per language, and enhances text wrapping to support multi-line text |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function switchHeadlineLanguage(lang) { | ||
| const text = getTextSettings(); | ||
| text.currentHeadlineLang = lang; | ||
| text.currentLayoutLang = lang; | ||
|
|
||
| // Load text for this language | ||
| document.getElementById('headline-text').value = text.headlines[lang] || ''; | ||
| // Sync text inputs and layout controls for this language | ||
| updateTextUI(text); | ||
| updateCanvas(); | ||
| } | ||
|
|
||
| function switchSubheadlineLanguage(lang) { | ||
| const text = getTextSettings(); | ||
| text.currentSubheadlineLang = lang; | ||
| text.currentLayoutLang = lang; | ||
|
|
||
| // Load text for this language | ||
| document.getElementById('subheadline-text').value = text.subheadlines[lang] || ''; | ||
| // Sync text inputs and layout controls for this language | ||
| updateTextUI(text); | ||
| updateCanvas(); | ||
| } |
There was a problem hiding this comment.
When switching headline or subheadline language, the currentLayoutLang is being set to the new language. However, if a language is later removed (in removeHeadlineLanguage or removeSubheadlineLanguage), and that language was set as currentLayoutLang, the currentLayoutLang won't be updated. Consider adding logic in the remove functions to also check and reset currentLayoutLang if it matches the removed language.
| if (screenshot) { | ||
| screenshot.text = normalizeTextSettings(screenshot.text); | ||
| return screenshot.text; | ||
| } | ||
| state.defaults.text = normalizeTextSettings(state.defaults.text); |
There was a problem hiding this comment.
The getText function normalizes text settings on every call by using JSON.parse(JSON.stringify(...)) operations, which are expensive. Since getText is called frequently through getTextSettings (including in input event handlers that fire on every keystroke), this could cause performance issues. Consider normalizing only once when text settings are initially loaded or modified, and storing a flag to track whether normalization has already been performed, to avoid redundant deep cloning operations.
| if (screenshot) { | |
| screenshot.text = normalizeTextSettings(screenshot.text); | |
| return screenshot.text; | |
| } | |
| state.defaults.text = normalizeTextSettings(state.defaults.text); | |
| if (screenshot) { | |
| // Normalize screenshot-specific text settings only once | |
| if (!screenshot.text || !screenshot.text._normalized) { | |
| screenshot.text = normalizeTextSettings(screenshot.text); | |
| // Mark as normalized to avoid redundant work on subsequent calls | |
| if (screenshot.text) { | |
| screenshot.text._normalized = true; | |
| } | |
| } | |
| return screenshot.text; | |
| } | |
| // Normalize default text settings only once | |
| if (!state.defaults.text || !state.defaults.text._normalized) { | |
| state.defaults.text = normalizeTextSettings(state.defaults.text); | |
| if (state.defaults.text) { | |
| state.defaults.text._normalized = true; | |
| } | |
| } |
Summary
Testing