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

Adding feature: distribute views along axis with maximum item length. #363

Closed

Conversation

gobetti
Copy link
Contributor

@gobetti gobetti commented Jul 8, 2016

I tried to keep this code as similar as possible to the current mas_distributeViewsAlongAxis methods. One of the biggest changes is that I moved the offset calculation to outside the for loop, as this value changes of a constant at every iteration, so it is not necessary to do a complex recalculation each time (increment or decrement by that constant is enough). In fact, I needed to use that constant to create a new constraint.

I recommend though a small refactor in the offset calculation for the other methods following the same approach.
untitled1
untitled2

@robertjpayne
Copy link
Member

@gobetti any chance you could write tests for this? It doesn't break anything else but just to be safe as I'm not 100% across what the intended functionality is.

@gobetti
Copy link
Contributor Author

gobetti commented Sep 14, 2016

Hi Robert, I probably can, will take a look at that during the week. It's related to issue #360 (created by me) and possibly #380.

@gobetti gobetti force-pushed the distribute-views-with-maximum-length branch from e076a46 to 21e2612 Compare September 14, 2016 23:25
@gobetti
Copy link
Contributor Author

gobetti commented Sep 14, 2016

Btw, I just noticed that this PR has two images attached demonstrating the functionality. The first image is from mas_distributeViewsAlongAxis, and the second is from the method introduced by this PR. Notice that in current Masonry's method, if one specifies a length that ends up being too large for the screen, the views will end up overlapping one another, as shown in the first image (any view at the right overlaps its neighbor left view).

In the method introduced by this PR, you specify a maximumLength instead of a fixedLength, and this method uses convenient Masonry's methods to ensure a final length such that no views will overlap one another, while still keeping all of them uniform.

I'll still create a test though, I should had done it from the beginning.

@gobetti
Copy link
Contributor Author

gobetti commented Sep 18, 2016

Hi @robertjpayne , I was studying the existent tests for NSArray+MASAdditions and in my opinion, the tests for the existent methods mas_distributeViewsAlongAxis:withFixedSpacing:... and mas_distributeViewsAlongAxis:withFixedLength:... are tests that don't really document what these methods were made for (testDistributeViewsWithFixed[Spacing|Length]ShouldFailWhenArrayContainsLessTwoViews and testDistributeViewsWithFixed[Spacing|Length]ShouldHaveCorrectNumberOfConstraints). If we want to create tests for demonstrating functionality, it seems that we'll need a whole new class of tests where a superview is updated according to the added constraints, and then the subviews can have their position and size properties tested.
I don't feel there will be much added value if I simply duplicate the existent tests for the method introduced by this PR.

@gobetti gobetti closed this Mar 18, 2019
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.

None yet

2 participants