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

Add deauthorize comfirmation dialog #1535

Merged
merged 1 commit into from Aug 25, 2019
Merged

Add deauthorize comfirmation dialog #1535

merged 1 commit into from Aug 25, 2019

Conversation

typebrook
Copy link
Contributor

@typebrook typebrook commented Aug 25, 2019

#1534

When user click on OAuth preference option and OSM account is authorized,
ask if user want to deauthorize it. If not, just do OAuth.

Screenshot_20190825-203737

When user click on OAuth preference option and OSM account is authorized,
ask if user want to deauthorize it. If not, just do OAuth.
@westnordost
Copy link
Member

Cool, thank you! I have a few comments, but I will just do this myself after the merge. Particularly, I want to change:

  • translation is done in POEditor and then always pulled from there. Will push the new strings to POEdtior and then edit in your strings.
  • first time I see takeIf in action. Though, in this case, I have the opinion that it does not increase readability (rather the contrary), I'll rather use a simple if-else here.
  • I will remove the string oauth_confirm_deauthroize_username. Since every string needs to be translated into 20+ languages, I weigh for every string if it is necessary
  • I want to improve the English wording

@westnordost westnordost merged commit b73c900 into streetcomplete:master Aug 25, 2019
@typebrook typebrook deleted the check-logout branch August 25, 2019 14:50
@typebrook
Copy link
Contributor Author

weigh for every string if it is necessary
readability

Good points! I'll take care of these rules next time.
Have a nice day!

@westnordost
Copy link
Member

You too!

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