Skip to content

Conversation

@ac-mmi
Copy link
Contributor

@ac-mmi ac-mmi commented Oct 25, 2025

The _ function in our i18n helper was unnecessarily converting text to title case as a fallback when a translation was missing. This caused issues with domain-specific strings like musical notes, codes, and proper nouns where the original casing should be preserved.

Changes made:

  • Removed the title-case fallback.
  • Ensures that if all translation attempts fail, the original text is returned unchanged.
  • No other logic has been altered, so casing is no longer modified unexpectedly.

Fixes #4779

Refactor translation logic to remove title case fallback.
@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@omsuneri
Copy link
Member

LGTM but the issue still persisting ???

@ac-mmi
Copy link
Contributor Author

ac-mmi commented Oct 25, 2025

@omsuneri output from my branch
Screenshot (3059)

@omsuneri
Copy link
Member

omsuneri commented Oct 25, 2025

Screenshot 2025-10-26 at 2 11 00 AM

are these changes explicitly for the phrase maker ?? cause when i m creating a note on canvas it is still there with "Ti" please verify once !!
also it is still showing undefined on collapsing so the issue is still there and the phrase maker is still rendering the ti note as Ti

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@ac-mmi
Copy link
Contributor Author

ac-mmi commented Oct 25, 2025

@omsuneri Removed previously added cleanedText fallback (added earlier by me) to preserve original text exactly and avoid unwanted changes for punctuation-sensitive strings like musical notes or codes.

@omsuneri
Copy link
Member

omsuneri commented Oct 26, 2025

Screenshot 2025-10-26 at 11 41 23 AM

issue is still there for the japanese follow the steps to reproduce in the issue by chnaging the language
look for it btw logic seems good to me in this case !!

@omsuneri omsuneri requested a review from Copilot October 26, 2025 06:19
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an issue where the i18n helper function was unintentionally modifying the casing of untranslated strings. The function previously attempted multiple fallback translation lookups with various text transformations (lowercase, title case, hyphenated), but when all translations failed, it would return title-cased text instead of the original. This caused problems with domain-specific strings like musical notes and codes that require specific casing.

Key changes:

  • Removed all fallback translation attempts that modified text casing (lowercase, title case, hyphenated variants)
  • Simplified the translation logic to preserve original text casing when no translation is found

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@ac-mmi
Copy link
Contributor Author

ac-mmi commented Oct 26, 2025

@omsuneri Updated _getPitch(c) in block.js to decouple logic lookup from localized display

  • Added solfnotesRef for canonical English solfege order (["ti","la","sol","fa","mi","re","do"]) used for indexing.
  • Continued using _('ti la sol fa mi re do') (solfnotesLocalized) for rendering the collapsed label, respecting translations.

@omsuneri
Copy link
Member

omsuneri commented Oct 26, 2025

@ac-mmi issue is not recurring anymore
Good Work !!

@omsuneri omsuneri self-requested a review October 26, 2025 20:15
Copy link
Member

@omsuneri omsuneri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@walterbender walterbender merged commit 5742a6e into sugarlabs:master Oct 27, 2025
5 checks passed
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.

"Ti" (with capital "T") in pie menu is causing problems for pitch blocks

3 participants