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

Improvements to globalization #67

Merged
merged 3 commits into from
May 8, 2023
Merged

Improvements to globalization #67

merged 3 commits into from
May 8, 2023

Conversation

United600
Copy link
Collaborator

@United600 United600 commented May 5, 2023

Moved manifest strings to another resource file for better organization, should help with localization.

Also added some strings (eg. "AppShortName", "AppDisplayName") for other writing systems.

ToDo

  • Move file logos assets to another folder

@huynhsontung
Copy link
Owner

huynhsontung commented May 5, 2023

I think this change is good. Moving these strings to a different resource file would remove codegen for them as well. The strings are only used in the manifest and not in code so I think it's safe. Thank you.

There are reports that file association icons are causing slowdown for some users. I will take care of them and move them to another folder.

@United600
Copy link
Collaborator Author

I think this change is good. Moving these strings to a different resource file would remove codegen for them as well. The strings are only used in the manifest and not in code so I think it's safe. Thank you.

Yes, easier to handle.

There are reports that file association icons are causing slowdown for some users. I will take care of them and move them to another folder.

Pretty sure it's not related to that but tbh not entirely happy with the icons, at least on 150 scale the 32px video downscales really bad. On this weekend I'll see what I can do.
Maybe don't even need one for each extension, with thumbnails I hardly see the bigger ones but it's always useful on list view, food for thought.

@United600
Copy link
Collaborator Author

United600 commented May 5, 2023

Just took a quick peek at the Media Player sizes and they have some weird ones.

imagem

The 32 pixel downscaled at 150 scale.
imagem

@huynhsontung
Copy link
Owner

The 32 pixel downscaled at 150 scale.

I suspect that icon is being dynamically downscaled. Since 32 at 100% is still quite legible. See #68 for more icon sizes that I have generated. Anything below 32px is going to be the simple icon.

@United600
Copy link
Collaborator Author

United600 commented May 6, 2023

yeah. I did a quick check for List View, so it goes something like this:

Scale Size
100 16
125 20
150 24
175 26 or 28?
200 32

@huynhsontung
Copy link
Owner

| 175 | 26 or 28? |

Native applications don't use 26 and 28. How did you verify this?

@United600
Copy link
Collaborator Author

United600 commented May 6, 2023

| 175 | 26 or 28? |

Native applications don't use 26 and 28. How did you verify this?

just changed the scale, log off, log in, open explorer.

and yes nobody uses 26 or 28, all downscaled the 32 pixel, my guess 175 is rarely used

is it worth it to make them? don't think so

@huynhsontung
Copy link
Owner

That's fine. Thank you for verifying. We will stick with what native applications use. They are:

128, 129, 16, 20, 24, 256, 30, 32, 336, 36, 40, 48, 60, 63, 64, 72, 80, 96

See #68 for more details.

With 5 main variations. The rest are scaled versions of these 5.

Copy link
Owner

@huynhsontung huynhsontung left a comment

Choose a reason for hiding this comment

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

Looks good. We can merge once you resolve the typo.

Screenbox/Package.appxmanifest Outdated Show resolved Hide resolved
scripts/update-manifest.ps1 Show resolved Hide resolved
@huynhsontung huynhsontung merged commit 1d4deb1 into huynhsontung:main May 8, 2023
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