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

fix(l10n): Use resource strings for default dialog string resources #92

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

nikclayton
Copy link
Contributor

The strings used in default dialogs were hardcoded. This meant they
couldn't be translated, or use the default platform strings (e.g.,
android.R.string.ok).

Fix that. Convert RegistrationDialogContent, NoDistributorDialog,
and ChooseDialog to interfaces, and provide implementations of those
interfaces that use the default strings.

Remove unnecessary API dependency on Kotlin stdlib.
The user might register with a distributor, and then delete the
distributor app.

If that happens the user is not prompted if there are no distributors,
and re-registration does not happen.

Fix this by checking the ack'ed distributor is still in the list of
installed distributors.
Previous code ignored the dialog's default layout and set its own
textview. I think this was so a LinkMovementMethod could be set on the
message.

This meant the view didn't follow normal Android dialog dimensions
(e.g., padding, margins, text layout), and looked out of place.

Fix this. Use the normal AlertDialog message. Set an onShowListener
that sets the LinkMovementMethod when the dialog is shown.
The strings used in default dialogs were hardcoded. This meant they
couldn't be translated, or use the default platform strings (e.g.,
android.R.string.ok).

Fix that. Convert `RegistrationDialogContent`, `NoDistributorDialog`,
and `ChooseDialog` to interfaces, and provide implementations of those
interfaces that
@p1gp1g
Copy link
Member

p1gp1g commented Aug 11, 2024

Thanks again for your contribution. This introduces breaking changes, so it can't be merged like it is

@nikclayton
Copy link
Contributor Author

Do you want me to add a commit to the PR that bumps the release version to 3.0.0?

@p1gp1g
Copy link
Member

p1gp1g commented Aug 12, 2024

I think it is better to keep that PR pending, we will update the specification in the coming months to embrace webpush. This will be an appropriate moment for a major release

@p1gp1g
Copy link
Member

p1gp1g commented Aug 12, 2024

The other option is to keep the object with a warning @ deprecated

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.

2 participants