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

scrubbed through app widgets and added localization text #493

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mytchj
Copy link
Contributor

@mytchj mytchj commented Aug 21, 2023

I added a few new English strings and descriptors to the en.arb file. Replaced some existing static strings in several of the widgets to utilize the L10n localization instead of static English strings.

child: Text('You are not logged in to any instances'),
? Center(
child: Text(L10n.of(context).not_logged_in),
//child: Text('You are not logged in to any instances'), // TODO localize this string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safe to remove the TODO now 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated with latest commit good catch haha

Copy link
Contributor

@mykdavies mykdavies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of TODOs left in the code -- are you intending to deal with these before we merge this PR?

Otherwise it looks like great work-- I know how much of a slog this can be, so thanks!

@mytchj
Copy link
Contributor Author

mytchj commented Aug 26, 2023

@mykdavies I have cleaned up the TODOs and added a few more localized strings. Should be good now. Thanks for the review

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