Skip to content

Conversation

@bobnik
Copy link
Collaborator

@bobnik bobnik commented Jan 2, 2026

This PR adds internationalization support to Sandify, adapted from @songfei's i18n implementation on songfei/master. All Chinese translations (~250 strings) are from @songfei's original work.

@jeffeb3, please weigh in on the approach and happy to adjust if needed.
@songfei, special thanks for your contribution. This is an awesome addition to Sandify. Let me know if I've missed anything in the adaptation.

What's included

  • i18next integration with React bindings
  • Chinese translations for all UI labels, options, tooltips, and shape descriptions
  • Language selector on the Settings tab
  • Browser language detection - auto-detects Chinese browsers, respects saved preference
  • Graceful fallback - missing translations display English

README.md details how to add future language support.

Approach

We use English strings as translation keys (e.g., t("Save")). This works well for Sandify since English is the primary development language - keys are readable, greppable, and missing translations gracefully fall back to the English key itself. For longer strings, we define semantic keys (e.g., t("description.heart")) and provide translations across all languages.

Not included (future PR)

Chinese FancyText fonts: The fonts are quite large, and highlight a larger issue of the application loading all fonts into memory upon load. We'll add a future PR with on-demand loading for fonts which will speed up application load time.

Testing

  • Switch language in Settings tab, verify UI updates
  • Refresh page, verify language persists
  • Clear localStorage, verify browser language detection works
  • Navigate between tabs after language change, verify patterns render

@bobnik bobnik changed the title Add i18n support with Chinese translations; based on i18n work by @songfei Add i18n support with Chinese translations (adapted from @songfei's PR) Jan 2, 2026
@songfei
Copy link

songfei commented Jan 4, 2026

I'm very happy with the changes based on my pull request. However, I noticed that this modification doesn't include the internationalization of the shapes and effects properties, which I had already implemented in my original code.

@songfei
Copy link

songfei commented Jan 4, 2026

We use English strings as translation keys (e.g., t("Save")). This works well for Sandify since English is the primary development language - keys are readable, greppable, and missing translations gracefully fall back to the English key itself. For longer strings, we define semantic keys (e.g., t("description.heart")) and provide translations across all languages.

Using words directly as keys is also a valid approach. However, the same word might be expressed differently in different UI locations and different languages. This is why I considered using module names as a hierarchical structure to organize the internationalization files.

@bobnik
Copy link
Collaborator Author

bobnik commented Jan 4, 2026

I noticed that this modification doesn't include the internationalization of the shapes and effects properties, which I had already implemented in my original code.

Right. I was trying to lean toward keeping your exact "coverage" of translations while simplifying maintenance here (35 vs 73 files changed). Regarding your other comment about a hierarchical key structure, we do a hybrid here - words as keys by default, and hierarchical keys whenever needed for semantic or other reasons.

@jeffeb3
Copy link
Owner

jeffeb3 commented Jan 7, 2026

First off, this is great! I'm really excited to see this and Thank you for adding this @songfei.

I really like the resulting code. It looks cleaner than I imagined after doing a translation. I'm a little worried the translation (and any future translations) will suffer from code rot and be out of date (I can't speak for Bob, but I won't be able to maintain these translations). I also want to make it clear that the code should remain in English (which you both get, but I want to make it clear for future contributors). I don't want someone offended when they do a bunch of work in another language and I have to reject it because I can't read it.

So I added a couple of sentences to the readme. I'm not trying to offend anyone, so let me know if I missed the tone.

Using words directly as keys is also a valid approach. However, the same word might be expressed differently in different UI locations and different languages

I would lean towards using the word as the translation until we have a conflict. If "radius" is fine and covers 10 locations, that will save a lot of headache. If "radius" is fine in 9/10 spots, but one location needs to be "shape.circle.radius", then we make that replacement there, and define it differently. This may end up being a fragile thing. If someone changes it because they need a different translation for Italian, and don't define the "shape.circle.radius" in Chinese. But until this is a problem, I'd like to use the simpler solution.

However, I noticed that this modification doesn't include the internationalization of the shapes and effects properties, which I had already implemented in my original code.

I'm not sure what this is referring to. I noticed the pre-populated names of layers and effects aren't translated:

Screenshot From 2026-01-07 10-41-13

@bobnik
Copy link
Collaborator Author

bobnik commented Jan 7, 2026

So I added a couple of sentences to the readme. I'm not trying to offend anyone, so let me know if I missed the tone.

Reads well to me.

@jeffeb3
Copy link
Owner

jeffeb3 commented Jan 8, 2026

I found another weird bug. I have only noticed this on this branch. If I change the language to Chinese, the stats go to 0/0.

It doesn't get fixed when I switch back to English. A refresh of the page fixes it. I don't see any errors in the console.

Screenshot From 2026-01-08 12-38-36 Screenshot From 2026-01-08 12-39-03

This looks good enough to merge. What order should I merge them in? I assume this is going to conflict with a lot of the other open PRs?

@bobnik
Copy link
Collaborator Author

bobnik commented Jan 8, 2026

@jeffeb3 I can look at the stats bug tomorrow most likely. Then, you should be able to merge this first and if there are conflicts afterward, I'll handle them. All PRs should (hopefully) be standalone.

@bobnik
Copy link
Collaborator Author

bobnik commented Jan 8, 2026

@jeffeb3 Added the zero stats fix to #331, so I'm merging this.

@bobnik bobnik merged commit dd68284 into master Jan 8, 2026
2 checks passed
@bobnik bobnik deleted the feature/i18n branch January 8, 2026 22:48
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.

4 participants