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

Localize site name/description for metadata preview pane usage #233

Merged
merged 1 commit into from Sep 6, 2019

Conversation

@wiz
Copy link
Member

commented Sep 6, 2019

Allows you to localize the preview text on Slack, Twitter, etc. like this:

Screen Shot 2019-09-06 at 10 02 08

@wiz wiz force-pushed the wiz:localized-metadata-for-preview branch 2 times, most recently from fda1118 to 6802d4c Sep 6, 2019
Copy link
Member

left a comment

utACK

@huey735

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

@m52go Could we change the English text to:
site_desc: "Bisq is an open-source desktop application that allows you to buy and sell bitcoins in exchange for national currencies or altcoins."

pt-PT translation
site_name: "Bisq - A exchange de Bitcoin descentralizada"
site_desc: "Bisq é uma aplicação de desktop open-source que lhe permite comprar e vender bitcoins em troca de moedas nacionais ou altcoins."

@wiz

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2019

why not "alternative crypto-currencies" ? too long?

@wiz wiz force-pushed the wiz:localized-metadata-for-preview branch from 6802d4c to 7578e47 Sep 6, 2019
@m52go

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

@wiz yeah for such site previews, every character counts. Would be too long, I think.

I would say it could be shortened even more to "Bisq is an open-source desktop application that allows you to trade bitcoin for national currencies or altcoins."

@wiz wiz force-pushed the wiz:localized-metadata-for-preview branch from 7578e47 to d7b5080 Sep 6, 2019
@wiz

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2019

The max length for opengraph description is 155 characters. The string "Bisq is an open-source desktop application that allows you to buy and sell bitcoins in exchange for national currencies, or alternative crypto currencies." is 154 characters so it fits perfectly, and I don't see any issue?

@m52go

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

No issues, but I would still say shorter is better, like how a 50-character tweet is more likely to be read entirely than a 240-character tweet.

I don't have a strong opinion here...it was just a suggestion. Maybe others can chime in.

@wiz

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2019

Of course, you need to fit your translated text within the 155 character limit, so by all means shorten your translations if necessary. I think it's important to say "alternative crypto-currency" and not "altcoins" because newbies might not know what those are.

@wiz wiz force-pushed the wiz:localized-metadata-for-preview branch from d7b5080 to 744d9e8 Sep 6, 2019
Copy link
Member

left a comment

test

EDIT: I suggested some changes but they didn't get added to my review. Will try to submit review again.

@wiz

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2019

you can't suggest changes if I already committed and pushed them 😉

Copy link
Member

left a comment

I prefer this wording.

_data/languages.yml Show resolved Hide resolved
_data/languages.yml Show resolved Hide resolved
_data/languages.yml Show resolved Hide resolved
@wiz

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2019

IMO your requested changes are out of scope for this PR. This PR is only to implement the ability to localize the description into various languages, not to change any strings. If you want to change strings, you should do that in your own PR, not in mine :)

@m52go

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

You're on thin ice here buddy. But I acknowledge this contribution 😄

@m52go

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

ACK

@m52go m52go merged commit 0ed8a60 into bisq-network:master Sep 6, 2019
@wiz wiz deleted the wiz:localized-metadata-for-preview branch Sep 6, 2019
@wiz wiz referenced this pull request Sep 9, 2019
@erciccione erciccione referenced this pull request Sep 9, 2019
@m52go m52go referenced this pull request Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.