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 way to change bar programmatically without triggering listeners #652

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

Conversation

narfman0
Copy link

@narfman0 narfman0 commented Feb 9, 2017

Currently the easiest way to change without triggering a listener is something like:

bottomBar.setOnTabSelectListener(null);
bottomBar.selectTabAtPosition(position, true);
bottomBar.setOnTabSelectListener(this, false);

This changeset exposes a way to avoid triggering listeners through the most likely call the user is already making. Previous API should behave the same, only users invoking this with triggerListeners (false) explicitly will see the difference.

Note: updateSelectedTab basically just calls listeners and updates currentTabPositon. I would have added triggerListeners there too, but it seems to be more like a setCurrentTabAndFireEvents :) so why nto just set current tab.

Fixes #499

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 57.244% when pulling d004792 on narfman0:master into 338ee86 on roughike:master.

@yombunker
Copy link
Collaborator

@narfman0 too many booleans already, in my opinion even one can sometimes result in ambiguous meaning.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 66.469% when pulling b96f6cc on narfman0:master into c7e0fb5 on roughike:master.

Copy link
Collaborator

@yombunker yombunker left a comment

Choose a reason for hiding this comment

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

Find a different way of doing this, there are too many booleans, and you could easily do the same on your side, set a boolean and when the callback comes back you ignore it and reset the boolean.

@narfman0
Copy link
Author

narfman0 commented Apr 4, 2017

You want another method, like selectTabAtPositionDontTriggerListeners? Not great method name :) but that kind of thing?

You suggest adding a boolean, managing my own state, and taking different logic based on internals of the bottom bar library that now I have to know about. All that over an overloaded method with an added boolean. Why would clients want this? Clunky, unclear, design I don't want in my app(s). You saw #499 ? Currently as I said, having a helper function clear/reset the listeners before/after updating is the cleanest way for me.

"too many booleans": Booleans are concise. triggerListeners doesn't sound ambiguous, though I would like more optimal suggestions. Assuming you just mean in a method signature e.g. trigger(false, false) I could see wondering the 1st vs 2nd false seeing it for the first time... for 3 seconds before hovering/single keystroke go to definition away from finding the name. By default, I'd expect clients to use the shorter methods, and not find this unless they need it when selecting tabs programmatically. Java is a verbose language; adding another method with more overloaded/specific/tailored functionality is expected and typical. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants