TiPresenter#runOnUiThread(Runnable) #65
Conversation
… all observers received the event
| @@ -73,6 +75,11 @@ public P getPresenter() { | |||
| return mDelegate.getPresenter(); | |||
| } | |||
|
|
|||
| @Override | |||
| public Executor getUiThreadExecutor() { | |||
| return new UiThreadExecutor(); | |||
There was a problem hiding this comment.
without knowing the exact context an understanding question: is it necessary to always create a new UiThreadExecutor ?
There was a problem hiding this comment.
It's harder to leak an Activity when the Executor isn't static or reused. Also it's a very small class, shouldn't cause problems
| /** | ||
| * Runs the specified action on the UI thread. It only works when a view is attached | ||
| * <p> | ||
| * When you are looking for a way to execute code when the view is attached have a look |
There was a problem hiding this comment.
When you are looking for a way to execute code when the view got available sometime in the future [...]
| * @param action the action to run on the UI thread | ||
| * @throws IllegalStateException when the executor is not available | ||
| */ | ||
| public void runOnUiThread(@NonNull final Runnable action) { |
There was a problem hiding this comment.
Please add some tests for this method.
Also when it is not really usable for "users"... But it is public. So we need to take care of it.
Also thinking about to make it just protected... Isn't it enough? 🤔
There was a problem hiding this comment.
sure, first of all I want an overall acceptance before I invest time to test his approach
There was a problem hiding this comment.
cannot be protected because the View implementer (Activity via delegate) has to set it, therefore it has to be public because they live in a different package.
| private final Executor mUiThreadExecutor; | ||
|
|
||
| public UiThreadExecutorAutoBinder(final TiPresenter presenter, | ||
| final Executor uiThreadExecutor) { |
There was a problem hiding this comment.
When we expect a UiThreadExecutor, then change the Executor signature to it.
Otherwise we can put any Executor to it which isn't on the UiThread.
There was a problem hiding this comment.
This would break our compatibility with JVM tests. The delegates and all used interfaces are android classes free. The UiThreadExecutor is 100% Android related.
| @@ -33,6 +35,11 @@ | |||
| P getRetainedPresenter(); | |||
|
|
|||
| /** | |||
| * @return {@link UiThreadExecutor} | |||
| */ | |||
| Executor getUiThreadExecutor(); | |||
There was a problem hiding this comment.
This is the same like the AutoBinder. Change it to UiTheadExecutor as return value...
| // reverse observer order for teardown events; first in, last out | ||
| for (int i = mLifecycleObservers.size() - 1; i >= 0; i--) { | ||
| mLifecycleObservers.get(i).onChange(newState, hasLifecycleMethodBeenCalled); | ||
| } |
There was a problem hiding this comment.
- add tests for correct call oder
|
@StefMa Ready. I already tested the sample and a real project and everything works fine |
Allows code to run on the UI thread within the
TiPresenter. This is most likely not a method for users ofTibut for extensions using theLifecycleObserver-API (another PR will follow).public API changes
The action of
TiPresenter#sendToView(Runnable action)will be now be called on the UI thread. Furthermore will those actions be executed afteronAttachView(TiView)and after all lifecycle observers have been called. This allows preparing the view inonAttachView(TiView)for those actions. That way the view should be in a "running" state as if the view was never gone.For very rare and special cases
TiPresenter#getQueuedViewActions()can be used to manually execute queued actions. People are creative.Another change has been made to
RxTiPresenterUtils#isViewReady()which was firing the ready event beforeonAttachView(TiView)was called. The ready event is now fired afteronAttachView(TiView)The execution order of lifecycle obersvers when receiving teardown events has been reversed; following "first in, last out"
internal changes
The method in
DelegatedTiActivity#postToMessageQueue(Runnable runnable)has been replaced withDelegatedTiActivity#Executor getUiThreadExecutor();, same for theFragmentinterface. All classes have been adjusted.I think the API changes are not critical and could also be bug fixes preventing rare race conditions.