Fix confusing setting label #1995
Conversation
Remove "in web client" from "audible notifications" setting label, since Riot is also a desktop application.
|
I think the catch there is it suggests that my phone will also not have audible notifications but I'm not sure it has that effect |
|
So the iOS and Android Riot clients do not use this setting? |
|
well they get notifications differently so they might not |
|
Oh, right. In either case, I think this is an improvement. |
I think I agree with @t3chguy here, it's ambiguous because the user might assume it affects all of their devices. If the point of this is to clarify the setting, then let's not leave it ambiguous. How about "Enable audible web/desktop notifications" |
Add "in web/desktop client" to clarify that this does not affect Riot mobile.
|
Is the fix here able to land, assuming any conflicts are addressed? I'd like to help if there's something I can do to push this forward. |
|
@aidalgol if we can get a sign off (as per the CONTRIBUTING.md) and the conflicts resolved, then we can merge this :) For sign off, a comment on this PR with |
src/i18n/strings/es.json
Outdated
| @@ -714,7 +714,7 @@ | |||
| "Riot does not know how to join a room on this network": "Riot no sabe cómo unirse a una sala en esta red", | |||
| "Set Password": "Establecer contraseña", | |||
| "Enable audible notifications in web client": "Habilitar notificaciones audibles en el cliente web", | |||
| "Off": "Desactivado", | |||
| "Off": "Apagado", | |||
src/i18n/strings/lt.json
Outdated
| @@ -109,7 +109,7 @@ | |||
| "Failed to set Direct Message status of room": "Nepavyko nustatyti tiesioginio pranešimo kambario būklės", | |||
| "Monday": "Pirmadienis", | |||
| "All messages (noisy)": "Visos žinutės (triukšmingas)", | |||
| "Enable them now": "Įjungti juos dabar", | |||
| "Enable them now": "Įgalinti juos dabar", | |||
There was a problem hiding this comment.
also a bit accidental? I think these kinds of language fixes should be picked up by weblate come release time.
There was a problem hiding this comment.
I was just trying to resolve merge conflicts when merging develop into my branch. I am quite likely out of date with the developer workflow practices.
|
Thanks for making this contribution a while back. It looks more work was needed here to remove the other language file changes, but nothing happened after that. Do you intend to clean it up? |
|
@jryans What do I need to do? Simply revert the changes to language files besides |
Yes, that should do it! |
|
I have reversed the unwanted changes to the language files, and remerged |
|
@aidalgol Hmm, I am still seeing many non-EN language files modified in the PR file view: https://github.com/matrix-org/matrix-react-sdk/pull/1995/files |
|
@jryans I'm at a loss as to how to properly resolve this, then. |
This clarifies that the notification settings only apply to the current device. This also tries to apply the spirit of matrix-org#1995 which wanted to remove "web" from the label, since there's also a desktop client.
|
No worries! I have tried to carry the spirit of this change forward over in #2828. Thanks again for the suggestion! |
This clarifies that the notification settings only apply to the current device. This also tries to apply the spirit of matrix-org#1995 (authored by @aidalgol) which wanted to remove "web" from the label, since there's also a desktop client.
Remove "in web client" from "audible notifications" setting label, since Riot is
also a desktop application.
Signed-off-by: Aidan Gauland aidalgol+ghpr@fastmail.net