Skip to content

refactor: split translation keys for about screen #845

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

Merged
merged 6 commits into from
Mar 11, 2025
Merged

Conversation

CyanVoxel
Copy link
Member

@CyanVoxel CyanVoxel commented Mar 7, 2025

Summary

This PR refactors the "About TagStudio" window by reorganizing its widgets and splitting the translation keys out from the single monolithic "about.content" key to several smaller keys. This was prompted due to several hyperlinks being present in the base string which should NOT be made accessible for translators to modify.

Changes

  • Refactored the UI widgets and layouts of about.py.
  • Fixed DPI of logo for hi-DPI screens
  • Split the "about.content" key into "about.description", "about.config_path", "about.documentation", "about.license", and more.
  • Added translation keys for the "Found" and "Missing" strings used in FFmpeg module checks.
  • Stopped proper names from being translated (e.g. "GitHub", "Discord", "FFmpeg").
  • Removed HTML tags from the about screen translation strings.
  • Removed the "help.visit_github" key as it's use was replaced by the current about screen keys.
  • Removed an errant <b> tag in the translation key "trash.dialog.permanent_delete_warning".

Tasks Completed

  • Platforms Tested:
    • Windows x86
    • Windows ARM
    • macOS x86
    • macOS ARM
    • Linux x86
    • Linux ARM
  • Tested For:
    • Basic functionality
    • PyInstaller executable

@CyanVoxel CyanVoxel added Type: Bug Something isn't working as intended Type: Translations Modifies translation keys or translation capabilities. Priority: Medium An issue that shouldn't be be saved for last Status: Blocked This issue or pull request is awaiting the outcome of another issue or pull request labels Mar 7, 2025
@CyanVoxel CyanVoxel added this to the Alpha v9.5.2 milestone Mar 7, 2025
@CyanVoxel CyanVoxel added the Status: Mergeable The code is ready to be merged label Mar 7, 2025
@CyanVoxel CyanVoxel moved this to 🍃 Pending Merge in TagStudio Development Mar 7, 2025
@CyanVoxel CyanVoxel removed the Status: Mergeable The code is ready to be merged label Mar 8, 2025
@CyanVoxel CyanVoxel moved this from 🍃 Pending Merge to 🚧 In progress in TagStudio Development Mar 8, 2025
@CyanVoxel CyanVoxel marked this pull request as draft March 8, 2025 08:14
@CyanVoxel CyanVoxel changed the title fix(translations): remove errant <b> tag refactor(translations): remove hyperlinks from translations Mar 8, 2025
@CyanVoxel CyanVoxel changed the title refactor(translations): remove hyperlinks from translations refactor: split translation keys for about screen Mar 8, 2025
@CyanVoxel CyanVoxel added Type: Refactor Code that needs to be restructured or cleaned up Priority: High An important issue requiring attention and removed Priority: Medium An issue that shouldn't be be saved for last labels Mar 8, 2025
@CyanVoxel CyanVoxel marked this pull request as ready for review March 8, 2025 10:33
@Computerdores
Copy link
Collaborator

I noticed that you split things like Konfigurations-Pfad: {config_path}<br> out into just Konfigurations-Pfad without the colon and placeholder. I believe we had in a previous case decided to include colons and such in translations because other languages may not use them. Is this just an oversight?

@CyanVoxel
Copy link
Member Author

I noticed that you split things like Konfigurations-Pfad: {config_path}<br> out into just Konfigurations-Pfad without the colon and placeholder. I believe we had in a previous case decided to include colons and such in translations because other languages may not use them. Is this just an oversight?

You're right - in this case I was trying to avoid a scenario where I needed to give several additional new translations strings with the colons and placeholders to translate, which would introduce several new entry points for placeholder errors. The maintenance overhead of and fixing these broken placeholders has started to really build up for me, and it's not where I want to be spending my time for this project.

I've now addressed this inconsistency by adjusting the styling of the system info section to no longer relying on colons. I did try for quite a while to get the config path field's background to use the minimum amount of space needed, however I guess that's a quirk of Qt and word wrapping that I've been unable to discover a solution for.
image

@CyanVoxel CyanVoxel moved this from 🚧 In progress to 🏓 Ready for Review in TagStudio Development Mar 8, 2025
@CyanVoxel CyanVoxel moved this from 🏓 Ready for Review to 👀 In review in TagStudio Development Mar 8, 2025
@CyanVoxel CyanVoxel removed the Status: Blocked This issue or pull request is awaiting the outcome of another issue or pull request label Mar 10, 2025
@CyanVoxel
Copy link
Member Author

Merging this now as it's needed to prevent conflicts with incoming translations, and is needed as a base before #854 can be addressed

@CyanVoxel CyanVoxel merged commit b0047b2 into main Mar 11, 2025
10 checks passed
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in TagStudio Development Mar 11, 2025
@CyanVoxel CyanVoxel deleted the bye-bye-b branch March 11, 2025 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High An important issue requiring attention Type: Bug Something isn't working as intended Type: Refactor Code that needs to be restructured or cleaned up Type: Translations Modifies translation keys or translation capabilities.
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants