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

Dynamic segment sizing #34

Merged
merged 9 commits into from Feb 5, 2018
Merged

Conversation

maxcampolo
Copy link
Contributor

@maxcampolo maxcampolo commented Dec 18, 2016

This PR adds support for dynamic segment sizing based on the content size of the segment. Instead of fixing the number of visible cells, each cell is a different size based on the length of its title (plus the image in the case of the imageBefore and imageAfter styles). The way to use it is to pass a SegmentioPosition option in the initializer, either .dynamic or .fixed(maxVisibleItems: 3). The fixed option is the same behavior that exists currently.
Changes:

  • Add SegmentioPosition enum with fixed and dynamic options
  • Replace maxVisibleItems with maxVisibleItems property in fixed case eg. .fixed(maxVisibleItems: 3)
  • Update Points extension with more parameters for dynamic layout calculations
  • Replace scrollToItem at index path calls with scrollRectToVisible in scrollToItemAtContext
  • Update itemInSuperview method for dynamic and fixed position cases
  • Separated method for calculating segment width
  • Added width property to SegmentioItem
  • Update example project

There are quite a few changes in this PR so if you want anything updated, changed, done differently, etc. please let me know and I’m happy to do it!

Ty for the awesome framework.

@maxcampolo
Copy link
Contributor Author

Hey @rmvz3 all these changes are in the branch called fix/label-layout. I'll just merge it over to my master eventually but you can use that branch for now.

@rmvz3
Copy link

rmvz3 commented Apr 5, 2017

@maxcampolo I've just realized. That's why I deleted my question. My fault ¯_(ツ)_/¯

Thank you

@adrianod1as
Copy link

No love for this PR?
Any plans to merge this with the master?

@Tykhonkov
Copy link
Contributor

Tykhonkov commented Sep 1, 2017

Hello @maxcampolo , thank you for a great work. I have tested your PR and found some issues with cell width

Preview

and also with wrong positioning of indicator

Preview

Can you fix them?

@maxcampolo
Copy link
Contributor Author

@Tykhonkov sure thing let me look at this. The first image looks correct, the cell sizes based on the size of the content contained in it, using the image width if there is no label. The second image indicator position is definitely wrong. Any specifics on reproducing? Thanks!

@maxcampolo
Copy link
Contributor Author

@Tykhonkov I pushed some updates that should fix these issues. I also updated the example project so to test you can just add the segmentioPosition: .dynamic option to the SegmentioBuilder call in ViewDidAppear(_:) of ExampleViewController.
image

Thanks for reviewing!

@Tykhonkov
Copy link
Contributor

Tykhonkov commented Sep 6, 2017

i'll test it

@NSMyself
Copy link
Contributor

Apologies for bothering, but may I ask if this PR is going to be merged any time soon?
I'll just use the specific branch for now in any case

@AndriiPetrovDev AndriiPetrovDev changed the base branch from master to feature/swift4_support November 1, 2017 11:20
@AndriiPetrovDev AndriiPetrovDev changed the base branch from feature/swift4_support to master November 1, 2017 11:20
@AndriiPetrovDev
Copy link
Contributor

@maxcampolo Could you please update your request by pulling last changes in our master branch ?

@maxcampolo
Copy link
Contributor Author

@AndrewPetrov okay I've merged the latest from master and updated the PR. Thanks for reviewing!

@AndriiPetrovDev
Copy link
Contributor

AndriiPetrovDev commented Nov 1, 2017

@maxcampolo I'm sorry but your fix still has some issues with highlighting
It appears when you tap half-hidden most right cell

http://joxi.ru/gmvjlP0FLgy41m

@maxcampolo
Copy link
Contributor Author

@AndrewPetrov sorry about that! This was just a bug calculating the position in the .fixed segment width option. Turned out to be misplaced if statement brackets. Should be fixed now. Thanks!

…n .fixed Segmentio position, remove warnings
@batjo
Copy link

batjo commented Dec 14, 2017

Could this please be merged?

@rnkyr rnkyr added this to ToDo in v 3.0 Jan 18, 2018
@thehavre thehavre moved this from ToDo to In Progress in v 3.0 Feb 1, 2018
@thehavre thehavre merged commit 7772467 into Yalantis:master Feb 5, 2018
@thehavre thehavre moved this from In Progress to Done in v 3.0 Feb 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v 3.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

9 participants