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 NativeCore, LocaleState, CDSHandler #2

Merged
merged 1 commit into from
Dec 17, 2020
Merged

Conversation

Petrakeas
Copy link
Contributor

Implemented NativeCore, LocaleState, CDSHandler.

Added an example where the SDK is used to wrap
the context of a service, "SimpleIntentService".

Copy link

@dbendilas dbendilas left a comment

Choose a reason for hiding this comment

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

Looks great! I've made an initial pass of the code. I'll try defer manual testing until after MemoryCache is also implemented.


interface CurrentLocalListener {

void onLocalChanged(Locale newLocale);

Choose a reason for hiding this comment

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

onLocalChanged -> onLocaleChanged. Also, how about you also pass the previous locale, so that clients can implement more complex business logic depending on the exact change?

Copy link
Contributor Author

@Petrakeas Petrakeas Dec 14, 2020

Choose a reason for hiding this comment

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

This interface is supposed to be used internally. The client is the one that sets the locale in the SDK, if they handle it themselves.

Choose a reason for hiding this comment

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

So, you mean that they know the previous locale and they can use it if they wish?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The SDK user is the ones who sets the locale and informs the SDK. So, it makes sense that they should keep the previous locale state if needed. Plus, this callback is used internally in the SDK and it's not meant to inform the SDK user of a locale change.


}

private LocaleState.CurrentLocalListener mCurrentLocalListener = new LocaleState.CurrentLocalListener() {

Choose a reason for hiding this comment

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

CurrentLocalListener -> CurrentLocaleListener, mCurrentLocalListener -> mCurrentLocaleListener

Comment on lines 1 to 45
<?xml version="1.0" encoding="utf-8"?>
<resources>
<string name="app_name">Transifex Application ελ</string>

<string name="demo_activity_name">Transifex demo activity ελ</string>

<string name="title">Transifex SDK ελ</string>
<string name="subtitle">String demo ελ</string>

<string name="hello_world">Hello World! ελ</string>

<string name="welcome_text">Welcome to our demo ελ</string>
</resources>

Choose a reason for hiding this comment

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

Is this file partially filled on purpose or is it WIP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's WIP.

Implemented NativeCore, LocaleState, CDSHandler.

Added an example where the SDK is used to wrap
the context of a service, "SimpleIntentService".
Copy link

@dbendilas dbendilas left a comment

Choose a reason for hiding this comment

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

Great job!

@dbendilas dbendilas merged commit 9afbbf4 into devel Dec 17, 2020
@dbendilas dbendilas deleted the petrakeas/cdshandler branch December 17, 2020 12:50
@wyngarde wyngarde mentioned this pull request Apr 15, 2021
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