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

CompositeClickListener doesn't function as expected #13

Open
PeterAttardo opened this issue Mar 4, 2018 · 1 comment
Open

CompositeClickListener doesn't function as expected #13

PeterAttardo opened this issue Mar 4, 2018 · 1 comment

Comments

@PeterAttardo
Copy link

CompositeClickListener maintains a list of View.OnClickListeners and when it receives a click event it iterates through the list backward, delegating the click to each in turn. The problem with this approach is that in the case of AbsMaterialCheckablePreference the preference sets itself as a listener in its onViewCreated method, which is the listener responsible for switching the value. Because this is the first listener in the CompositeClickListener's list, it is the last to be executed, so all other click listeners will be executed using the old value of the preference should they call getValue().

This is particularly notable with MaterialPreferenceScreen#setVisibilityController as it sets a click listener on the controller MaterialPreference which is consistently invoked with the wrong boolean value. The sample app demonstrates this behavior (despite what appears to be an attempt to correct for it by setting showWhenChecked to false when it really wants to be true). When you enter the AppConfigActivity, the visibility of Location will be the opposite of what Automatic Location would indicate (because at this point it has the correct value and is respecting showWhenChecked = false. When you switch the value for the first time, the visibility will not change (as it is still using the old value).

Possible solutions include:

  • Invert controller.getValue() and !controller.getValue() at line 73 of MaterialPreferenceScreen
  • Iterating through the CompositeClickListener forward instead of backward
  • Creating a separate OnPreferenceChangedListener and relying on that for setVisibilityController (this could be more broadly useful as well)
  • Having AbsMaterialCheckablePreference set a composite CompositeClickListener which always delegates to the AbsMaterialCheckablePreference first.
@anggrayudi
Copy link

It is been 3 months, and no fixes yet?

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

No branches or pull requests

2 participants