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

Update zh-Hans Translation and Fix selectedInterface #259

Merged
merged 6 commits into from
Jan 10, 2021
Merged

Update zh-Hans Translation and Fix selectedInterface #259

merged 6 commits into from
Jan 10, 2021

Conversation

Tai-Zhou
Copy link
Contributor

The words shown in MenuItem now can be localized. I also changed the name of fans which was like "Fan #0". Sorry for the inconvenience that my editor deleted spaces at the end of each line automatically.

@exelban
Copy link
Owner

exelban commented Dec 29, 2020

Please remove all redundant +- empty lines. I don't see what you try to change.

@exelban
Copy link
Owner

exelban commented Dec 29, 2020

Also, I noticed that you try to just translate values like At start. It cannot be done like this. These 2 select options (Update intervals and colors) not prepared for translations. There must be 2 separate values: one for key and another to display. I haven't done it yet.

And at this stage, you can commit the translations. But they will be not visible now.

@Tai-Zhou
Copy link
Contributor Author

Also, I noticed that you try to just translate values like At start. It cannot be done like this. These 2 select options (Update intervals and colors) not prepared for translations. There must be 2 separate values: one for key and another to display. I haven't done it yet.

Sorry for bothering, I've seen my mistakes. The spaces were added back and some changes reverted. The modification is now limited to the "Disabled" of the battery notification, the "Autodetection" of network interface and the name of fans.

@@ -110,7 +110,7 @@ internal class Settings: NSView, Settings_v {
}

@objc private func changeUpdateInterval(_ sender: NSMenuItem) {
if sender.title == "Disabled" {
if sender.title == LocalizedString("Disabled") {
Copy link
Owner

Choose a reason for hiding this comment

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

Sorry, but the saving key cannot be translated. It must be constant.

Copy link
Contributor Author

@Tai-Zhou Tai-Zhou Dec 30, 2020

Choose a reason for hiding this comment

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

Thank you for your review! I just made a new commit to keep the lowLevelNotification and saving term Battery_lowLevelNotification with a constant value "Disabled".

@@ -123,7 +123,7 @@ internal class Settings: NSView, Settings_v {
@objc func handleSelection(_ sender: NSPopUpButton) {
guard let item = sender.selectedItem else { return }

if item.title == "Autodetection" {
if item.title == LocalizedString("Autodetection") {
Copy link
Owner

Choose a reason for hiding this comment

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

The same problem as in the Battery module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if item.title == "Autodetection" {
self.store.pointee.remove("\(self.title)_interface")

When item.title == "Autodetection", the term Network_interface would just be removed, so I think saving state would not be changed.

@exelban
Copy link
Owner

exelban commented Jan 4, 2021

Hi.
I still see that you try to translate not prepared values (LocalizedString("Disabled") and LocalizedString("Autodetection") ). I will not merge it. See my second post why.

@Tai-Zhou
Copy link
Contributor Author

Tai-Zhou commented Jan 5, 2021

Happy New Year! 🎇🎆
I just realized I've misunderstood your meaning of "key", I thought it was the key of storage 😅

if selectedInterface == "" {
self.button?.selectItem(withTitle: "Autodetection")
}
self.button?.menu = menu

I made a new commit for LocalizedString("Autodetection"). Now both the selection and storage don't depend on the localization anymore. So I think defining a [KeyValue_t] would be a little bit redundant for this part. I also find the line 115 in the code block above should be moved before selectItem to make it work properly.
For LocalizedString("Disable"), it is indeed not a good modification, I can revert this part.

@exelban
Copy link
Owner

exelban commented Jan 9, 2021

Hi. Sorry for the long response.

Please do not translate the values which not ready for translation. If you want to have these values translated you can help to make it ready. You can take a look here how I make select option translatable. It must have a key, which will be used to store. And value (text) which could be translated.

@Tai-Zhou Tai-Zhou changed the title Add Localization to MenuItem and Fan. Update zh-Hans Translation and Fix selectedInterface Jan 9, 2021
@Tai-Zhou
Copy link
Contributor Author

Tai-Zhou commented Jan 9, 2021

I see, that's very kind of you with great patience, and sorry for troubling you for such a long time. I've reverted all the changes except the selectedInterface, and a new version of zh-Hans translation is updated.

@exelban exelban merged commit 44adf3a into exelban:master Jan 10, 2021
gmcinalli pushed a commit to gmcinalli/stats that referenced this pull request Feb 28, 2022
* Add Localization to MenuItem and Fan.

* Add spaces back.

* Revert changes of L10n update-interval and color.

* Fix storage of "Disabled".

* Update the way to L10n of "Autodetection"

* Revert Changes
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.

None yet

2 participants