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

Fix music images items in other languages #1014

Merged

Conversation

THEb0nny
Copy link
Contributor

@THEb0nny THEb0nny commented Apr 27, 2023

The problem has existed for a long time, icons for elements with music are not displayed.
If you look, then in Russian an undenfined error appears in the console.
buttonImg.src = this.getSoundIcon(category);

Here are some examples. English language.
image

That's it in Russian.
image
(These are pictures from the beta version.)

The problem was that if you enable another language on the site (for example, Russian), then the options values were taken with a translation in the selected language. They didn't match the category values (in English) in icons.jres.

  1. Therefore, I decided to change it so that options sorting is disabled so that options do not change places in different languages.

  2. Next, I from the initially obtained values
    soundIconCache = JSON.parse(pxtTargetBundle.bundledpkgs['music']['icons.jres']);

image

I recycle the object so that all elements are with indices
soundIconCacheArray = Object.entries(soundIconCache);

I remove the first element from my array, it is not needed. Interferes.
soundIconCacheArray.shift();

  1. In the refreshOptions function, I first get all the categories.
    const categories = this.getCategories(options);
    Then I look up the index in the array with all the categories of the category element
    const categoryIndex = categories.indexOf(category);

Further, to set the picture for the musical element, I pass this index to getSoundIcon.
buttonImg.src = this.getSoundIcon(categoryIndex);

  1. I rewrote the getSoundIcon function itself so that it takes an icon from my earlier soundIconCacheArray by index.

I checked in several languages. The changes are working.
As a result, we get this...
image

Correction of displaying pictures in different languages. The category options values were translated into different languages and the translated values could not be checked against the values from icon.jres.
Уlement layout fixes, for more readable blocks
@THEb0nny
Copy link
Contributor Author

THEb0nny commented Apr 27, 2023

I also made some improvements to the interface.

For example, I expanded them, made them always the same height, did work on the text.

It was like this before the change...
image
image
image

Now with changes...
image
image
image

@THEb0nny
Copy link
Contributor Author

THEb0nny commented Apr 27, 2023

@jwunderl But I have another question. I want to make it so that long words are wrapped with a hyphen.
I added style.hyphens = "auto" but it works if the correct language is in the html lang tag. It is now always displayed as lang="en". And hyphens doesn't work.
Another way is if the tag does not specify lang="ru". Therefore, I have a question, how to find out the language that is included on the site in order to write it into the code in the tag?

I want to do something so that the text in Russian translation is displayed even better.

@THEb0nny
Copy link
Contributor Author

@jwunderl please see changes

fieldeditors/field_music.ts Outdated Show resolved Hide resolved
fieldeditors/field_music.ts Outdated Show resolved Hide resolved
THEb0nny and others added 2 commits May 2, 2023 10:57
Co-authored-by: Joey Wunderlich <jwunderl@users.noreply.github.com>
Co-authored-by: Joey Wunderlich <jwunderl@users.noreply.github.com>
@THEb0nny
Copy link
Contributor Author

THEb0nny commented May 2, 2023

@jwunderl indeed pxt.Util.userLanguage() works. I also found this in other files, but did not have time to try to apply it here.
I have one thought, maybe this tag should not be added for en, if it is already in the html tag?

If this English conditional change doesn't work for some reason, you can undo it...

The tag is not needed for English, because in the html tag it is already always set.
@THEb0nny
Copy link
Contributor Author

THEb0nny commented May 2, 2023

@jwunderl I checked, everything works. This change can be considered.

@THEb0nny THEb0nny requested a review from jwunderl May 2, 2023 08:27
Copy link
Member

@jwunderl jwunderl left a comment

Choose a reason for hiding this comment

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

I had thought I merged / bumped this one, must have gotten distracted, sorry! Thanks for the pr!

@jwunderl jwunderl merged commit 2ca706d into microsoft:master May 5, 2023
@THEb0nny THEb0nny deleted the fix-music-images-items-in-other-languages branch May 6, 2023 10:18
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.

2 participants