Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Unlocalizable strings #3045

Closed
sohomang opened this issue Oct 24, 2019 · 8 comments
Closed

Unlocalizable strings #3045

sohomang opened this issue Oct 24, 2019 · 8 comments
Assignees
Labels
type: bug 🐛 Something isn't working
Milestone

Comments

@sohomang
Copy link
Member

sohomang commented Oct 24, 2019

Description

The following strings do not exist in translations/en.json, so they're not showing up in Crowdin:

  • "Channels" menu's label on the main screen
  • "onchain" and "in channels" on the Pay screen
  • "or" on the Connect screen during the onboarding
  • Some error message that shows up when trying to unlock a remote node

Bonus:

  • Shouldn't the open status label next to each channel say online instead? In contrast to the offline status.

Screenshots

Respectively:
Channels
On-chain:In channels
Or
Screen Shot 2019-10-24 at 02 33 42
Screen Shot 2019-10-24 at 02 36 35

@bolatovumar
Copy link
Contributor

I can take a look and include missing strings if they are hardcoded.

bolatovumar added a commit to bolatovumar/zap-desktop that referenced this issue Oct 25, 2019
address "Channels menu's label on the main screen" issue in LN-Zap#3045
bolatovumar added a commit to bolatovumar/zap-desktop that referenced this issue Oct 25, 2019
address '"onchain" and "in channels" on the Pay screen' issue in LN-Zap#3045
bolatovumar added a commit to bolatovumar/zap-desktop that referenced this issue Oct 25, 2019
address '"or" on the Connect screen during the onboarding' issue in LN-Zap#3045
@bolatovumar
Copy link
Contributor

@sohomang I addressed all issues except for the last one and the "bonus" one. I didn't address the last one

Some error message that shows up when trying to unlock a remote node

As these are error messages, do we wanna have them localizable @mrfelton?

As for the bonus one

Shouldn't the open status label next to each channel say online instead? In contrast to the offline status.

This makes sense to me. Is there a reason why we would want to keep it as "open" instead of "online" @mrfelton?

bolatovumar added a commit to bolatovumar/zap-desktop that referenced this issue Oct 26, 2019
address "Channels menu's label on the main screen" issue in LN-Zap#3045
bolatovumar added a commit to bolatovumar/zap-desktop that referenced this issue Oct 26, 2019
address '"onchain" and "in channels" on the Pay screen' issue in LN-Zap#3045
bolatovumar added a commit to bolatovumar/zap-desktop that referenced this issue Oct 26, 2019
address '"or" on the Connect screen during the onboarding' issue in LN-Zap#3045
bolatovumar added a commit to bolatovumar/zap-desktop that referenced this issue Oct 26, 2019
address '"onchain" and "in channels" on the Pay screen' issue in LN-Zap#3045
@bolatovumar
Copy link
Contributor

Shouldn't the open status label next to each channel say online instead? In contrast to the offline status.

According @sohomang, there was a consensus that we want to change the wording to "online" here. Should we go ahead with it @mrfelton?

@bolatovumar
Copy link
Contributor

Also, I'm wondering if we should have a lint rule to disallow hardcoded strings in JSX to avoid this sort of bug in the future. I found this rule: https://github.com/Shopify/eslint-plugin-shopify/blob/master/docs/rules/jsx-no-hardcoded-content.md

There might be rules in other packages that do the same thing.

Thoughts, @mrfelton?

@mrfelton mrfelton added the type: bug 🐛 Something isn't working label Oct 26, 2019
@mrfelton
Copy link
Member

According @sohomang, there was a consensus that we want to change the wording to "online" here. Should we go ahead with it @mrfelton?

I think that makes sense - what we call open in this case actually means open+online, so online would be a more accurate description since a channel can be open+offline.

So, we'd have:

online (open and online)
offline (open but offline)

Also, I'm wondering if we should have a lint rule to disallow hardcoded strings in JSX to avoid this sort of bug in the future. I found this rule: https://github.com/Shopify/eslint-plugin-shopify/blob/master/docs/rules/jsx-no-hardcoded-content.md

I don't know of other lint rules to do this @bolatovumar . I'd definitely be interested to see if this package works well as this is a common problem and one thats too easy to overlook.

bolatovumar added a commit to bolatovumar/zap-desktop that referenced this issue Oct 26, 2019
@bolatovumar
Copy link
Contributor

@mrfelton ok, i tried out that rule and it found 26 errors. I'm thinking it's worth it to create a separate issue for this and have a PR that enables this rule?

@mrfelton
Copy link
Member

ok, i tried out that rule and it found 26 errors. I'm thinking it's worth it to create a separate issue for this and have a PR that enables this rule?

Yes, please do @bolatovumar

@mrfelton mrfelton added this to the v0.6.0-beta milestone Oct 27, 2019
@mrfelton
Copy link
Member

Closed via #3050

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug 🐛 Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants