search

novoda / landing-strip

Your simple sliding viewpager tab strip: a landing strip without the fluff!
Apache License 2.0
13 stars 5 forks source link

V2 - Modularising the OnPageChangeListener #33

Closed ouchadam closed 7 years ago

ouchadam commented 7 years ago

Following the outcome of https://github.com/novoda/landing-strip/pull/32#discussion_r107132578

Removes the OnPagechangeListener from the LandingStrip, means exposing the mutableState and tabsContainerView via package local getters.

Api becomes

ViewPager.OnPageChangeListener landingStripPageChanger = LandingStripPageChangeListener.create(landingStrip);
viewPager.addOnPageChangeListener(landingStripPageChanger);

We'll eventually want to create a helper attacher to hide all of this

ataulm commented 7 years ago

ruhroh I merged in the wrong order

ouchadam commented 7 years ago

@ataulm nah they would have both conflicted

ataulm commented 7 years ago

Can LandingStrip and the ScrollingThing from the previous pr comment implement the ViewPager.OnPageChangeListener.

That way you don't need to expose the ScrollingPageListener which was the original beef, but you also don't need to create a LandingStripPageChanger?

edit: that's how it was originally -> I preferred :grin:

rock3r commented 7 years ago

Ok @ataulm my beef in the original discussion was due to the fact I didn't relise we are supposed to attach LandingStrip as a listener itself. I thought we could make it not implement the interface and use the ScrollingPageChangeListener directly. Given that, I'm ok with the current implementation, leaking less information outside. My bad, sorry for the wrong comments.

ataulm commented 7 years ago

:+1:

On Thu, 23 Mar 2017, 10:12 Sebastiano Poggi, notifications@github.com wrote:

Ok @ataulm https://github.com/ataulm my beef in the original discussion was due to the fact I didn't relise we are supposed to attach LandingStrip as a listener itself. I thought we could make it not implement the interface and use the ScrollingPageChangeListener directly. Given that, I'm ok with the current implementation, leaking less information outside. My bad, sorry for the wrong comments.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/novoda/landing-strip/pull/33#issuecomment-288673379, or mute the thread https://github.com/notifications/unsubscribe-auth/ACjfG4VAyTeJihWfbsulWhgVFMQu2dWZks5rokVygaJpZM4Ml0c8 .

-- Disclaimer: The information in this e-mail and any attachments is the property of Novoda Ltd and is confidential and may be legally privileged. This e-mail is intended solely for the person or organisation to which it is addressed. Any disclosure, copying or other use of the information by any person or organisation who is not the intended recipient is strictly prohibited and may be unlawful. If you have received this e-mail in error, please inform the sender immediately and delete/destroy this e-mail and any copies of it. Novoda Ltd has taken reasonable precautions to minimise the risk of any software viruses which may damage your systems, but we advise that you take the necessary steps to ensure that no virus contamination is suffered. Novoda Ltd does not accept any liability for any loss or damage caused by the transmission of any virus.

Novoda Ltd, Company No: 347444, Registered in Scotland Registered Office: C/O Alexander Sloan, 38 Cadogan Street, Glasgow, G2 7HF, Scotland. VAT Registration Number GB 984 2525 93

ouchadam commented 7 years ago

Going to close this, let's see how the current impl works out