-
Notifications
You must be signed in to change notification settings - Fork 65
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
Added translations for the SSO Support #152
Conversation
…registration_application_pending"
@dessalines @SleeplessOne1917 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides my other comments, you're missing the translation for "cannot_view_secret_after_saving"
.
@@ -429,6 +430,7 @@ | |||
"invalid_username": "Invalid username.", | |||
"admin_already_created": "Sorry, there's already an admin.", | |||
"user_already_exists": "User already exists.", | |||
"username_already_exists": "Username already exists.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a meaningful difference between this and user_already_exists
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Username is just more clear given that there's a Username
input field that the user is submitting. User
could include email too.
translations/en.json
Outdated
"oauth_client_secret": "Client Secret", | ||
"oauth_scopes": "OAuth Scopes", | ||
"oauth_auto_verify_email": "Auto-Verify Email", | ||
"oauth_account_linking_enabled": "Account Linking Enabled", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Enable Account Linking" reads a bit more clearly imo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
"no_oauth_providers_blurb": "No providers", | ||
"delete_oauth_provider_are_you_sure": "Are you sure you want to delete this provider?", | ||
"deleting_oauth_provider": "Deleting provider...", | ||
"oauth_display_name": "Display Name", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually already have a "display_name"
entry with the same value. Unless it's possible that this will differ from that one in other languages, we should use the existing one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We made them separate for this exact purpose.
translations/en.json
Outdated
"oauth_scopes": "OAuth Scopes", | ||
"oauth_auto_verify_email": "Auto-Verify Email", | ||
"oauth_account_linking_enabled": "Account Linking Enabled", | ||
"oauth_enabled": "Enabled", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, this controls whether users can actually use a given provider? In that case, "Enable Provider" might make things less vague.
Also, should OAuth providers that aren't enabled be shown to clients on the login screen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only enabled providers should be shown to clients. It's already the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
"couldnt_create_oauth_provider": "Couldn't create OAuth provider.", | ||
"or": "Or", | ||
"available_oauth_providers": "Login with", | ||
"oauth_login_with_provider": "Login with SSO Provider", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might need to remove oauth_login_with_provider
depending on this potential change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think keeping oauth_login_with_providers
is good to keep. If there's anything we need to remove, it's available_oauth_providers
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh this was part of the modal. Removed it.
|
@SleeplessOne1917 the translations are ready if there are no more changes on the UI side. |
Added the translations for the SSO Support