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

buttonBarView #375

Closed
wants to merge 4 commits into from
Closed

buttonBarView #375

wants to merge 4 commits into from

Conversation

codwam
Copy link

@codwam codwam commented May 14, 2017

BaseButtonBarPagerTabStripViewController.swift:
buttonBarView in code crash

codwam added 4 commits May 14, 2017 15:56
BaseButtonBarPagerTabStripViewController.swift:
buttonBarView in code crash
Add "badge" and "userInfo" as extra properties?
Delete repeat init method
IndicatorInfo add full initialize
@gonghao
Copy link

gonghao commented May 16, 2017

It's very useful! Hope to merge into master! 👍

Copy link
Contributor

@santiagofm santiagofm left a comment

Choose a reason for hiding this comment

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

We totally missed the error with the weak reference, good catch !
But i'd rather we have the same thing that was done in ButtonBarPagerTabStripViewController so if you can change it i'll merge it.
Also, please remove the changes to the IndicatorInfo that seem to be related to some custom implementation of a badge and don't forget to squash everything into one commit.

Cheers

@gonghao
Copy link

gonghao commented May 18, 2017

@santiagofm Maybe we should make a discussion about IndicatorInfo in #378

@codwam
Copy link
Author

codwam commented May 18, 2017

@santiagofm
1.
The weak properties you can modify, it's easy.
2.
The IndicatorInfo extension some properties is needed.
Firstly: The Youtube example is simple, and my cell has more than three labels !!!
Seondly: extenstion IndicatorInfo then use runtime to extension is no effect, case it's struct !!!

@santiagofm
Copy link
Contributor

@gonghao Sure

@codwam Yes, i understand. Still, i think it would be better to have this PR fix the weak buttonBarView issue so we merge it immediately and discuss what to do with IndicatorInfo in #378.

@codwam
Copy link
Author

codwam commented May 19, 2017

I'm not familiar with Pull Request,what should I do?

@santiagofm
Copy link
Contributor

@codwam I think squashing those 4 commits into just one and then amending it to remove the IndicatorInfo stuff you should be good to go

@codwam
Copy link
Author

codwam commented May 20, 2017

Sorry, I don't know how to revert.

@codwam codwam closed this May 20, 2017
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.

3 participants