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

Rename "Spread" tab to a more general "Details" #3745

Merged
merged 1 commit into from Dec 7, 2019
Merged

Conversation

@dmos62
Copy link
Contributor

dmos62 commented Dec 4, 2019

Fixes #3376

Because spread is one of many columns in the table in the "Spread" tab, calling the whole tab "Spread" is not general enough. Calling the table "Details" is more general and is meant to signify that it offers more information than the "offer book" view.

A counter-argument would be that 1) the table does not actually offer new information, just new aggregates on the same information displayed under "Offer book"; and 2), "Details" is somewhat vague.

Currently, it is unclear what a more accurate title could be, and in the meantime "Details" is an improvement on "Spread".

@dmos62 dmos62 requested a review from ripcurlx as a code owner Dec 4, 2019
Copy link
Contributor

julianknutsen left a comment

Testing

The english version looks good.
verify_details

The Spanish version still has the old text which I think is to be expected. I have no experience with how the translations are done, so someone with a better understanding of the workflow can weigh in here.

verify_details_spanish

The fact that this was marked as a "Good First Issue" makes me think it is OK for just the english version to change, but @ripcurlx or @freimair can clear that up.

As far as your analysis on whether or not this is the "best" text. I think that this issue has been up for review for almost two months now so any sort of bikeshedding on it should have happened. The new text is definitely moving it in the right direction imo.

@dmos62

This comment has been minimized.

Copy link
Contributor Author

dmos62 commented Dec 4, 2019

I was wondering about translation as well. In the localisation docs it says that the localised files should not be modified directly, but through the third-party translation platform.

@dmos62 dmos62 force-pushed the dmos62:master branch from 748c054 to 7e5422f Dec 5, 2019
Because spread is one of many columns in the table, calling the whole tab spread is not general enough.
@ripcurlx

This comment has been minimized.

Copy link
Member

ripcurlx commented Dec 6, 2019

@dmos62 @ julianknutsen Only the English translation is to be changed on master. If the value for one key is changing it is marked untranslated on Transifex and translators can pick it up for translation. Before each release I'm pulling the translations from Transifex and update it in the repository.

@ripcurlx

This comment has been minimized.

Copy link
Member

ripcurlx commented Dec 6, 2019

So we do update translations on master, but it is only the release manager (me) atm who is doing this to prevent manual changes of the translation files by mistake.

@ripcurlx

This comment has been minimized.

Copy link
Member

ripcurlx commented Dec 6, 2019

Regarding the new Details label: @m52go Do you have an opinion on this?

JFYI: @m52go is our proof reader when an English translation is added or changed.

@sqrrm
sqrrm approved these changes Dec 6, 2019
Copy link
Member

sqrrm left a comment

ACK

I think this wording is an improvement. Still will let @m52go have a say before merging.

@m52go

This comment has been minimized.

Copy link
Member

m52go commented Dec 7, 2019

Looks good to me 👍

@sqrrm sqrrm merged commit b2ea064 into bisq-network:master Dec 7, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@julianknutsen

This comment has been minimized.

Copy link
Contributor

julianknutsen commented Dec 7, 2019

@dmos62 Congrats on your first merged PR!

@ripcurlx ripcurlx added this to the v1.2.5 milestone Dec 10, 2019
@sqrrm sqrrm mentioned this pull request Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.