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

[WIP] Adding baseline align support #14

Closed
wants to merge 1 commit into from

Conversation

Thomvis
Copy link
Contributor

@Thomvis Thomvis commented Jun 23, 2015

I am working on adding baseline align support, because I needed it. This requires a bit of a change in the way the alignment strategies are called, because the alignment is between views and not between the view and the stack view.

I'm looking for feedback on the following points:

  • Is this how UIStackView baseline aligns labels? (haven't tried it myself yet)
  • What do you think about the way I've extended the alignment strategy?
  • To test this, new (UILabel based?) tests are needed. How do you want the test suite to grow?

@nsomar
Copy link
Owner

nsomar commented Jun 24, 2015

This looks amazing 👍
Do you think you will be able to add some unit test to cover these enums? If not, then I can add them on the weekend.

Thanks for your great addition 👍 !!

@Thomvis
Copy link
Contributor Author

Thomvis commented Jun 27, 2015

#17 now contains a fix for an issue I discovered yesterday with this PR. Sorry that they're so intertwined. Let me know if I can help.

I won't have time this weekend, so feel free to merge to your likings.

@nsomar
Copy link
Owner

nsomar commented Jul 4, 2015

Hey,
Sorry for being super late. I just reviewed this PR and it looks very cool.
However, I ran a couple of tests on UIStackView and it seems that the baseline constraints works as following:

For this example consider the following image

screen shot 2015-07-05 at 00 55 05

In your implementation you are adding baseline constraint for button 2 in relation to 1, and button 3 in relation to 2, etc..
However, In UIStackView, baseline constraint are always added to the first view. So button 2 adds to 1 and button 3 to 1 etc..

The following is a po of the constraints added in UIStackView:

<NSLayoutConstraint:0x7f984ac40510 'UI-alignment' UIButton:0x7f984ad0e310'1'.lastBaseline == UIButton:0x7f984ad11670'2'.lastBaseline>,
<NSLayoutConstraint:0x7f984ac42f80 'UI-alignment' UIButton:0x7f984ad0e310'1'.lastBaseline == UIButton:0x7f984ad11e80'3'.lastBaseline>,
<NSLayoutConstraint:0x7f984ac43740 'UI-alignment' UIButton:0x7f984ad0e310'1'.lastBaseline == UILabel:0x7f984ad12460'adasdsdsadasdasda'.lastBaseline>,

I think we should consider implementing the same idea as with UIStackView. What is your input on this?
Also, what is your general idea on what unit tests can we use to assert the constraints? should we go with frames or with constraints?

@nsomar
Copy link
Owner

nsomar commented Jul 28, 2015

@Thomvis Hey, I will be trying to merge both your great PR's this weekend. About my last question. If you dont have free time, I can totally tackle it.

@Thomvis
Copy link
Contributor Author

Thomvis commented Jul 29, 2015

@oarrabi sorry, but I won't have time soon to improve this further. I do hope that we can eventually merge more changes from https://github.com/Thomvis/OAStackView back to this repo, but I understand that this will take time from me (if you agree with those changes ;)).

@nsomar
Copy link
Owner

nsomar commented Aug 8, 2015

Thanks for the addition, I added some specs and merged this PR, Wonderfull work!

@nsomar nsomar closed this Aug 8, 2015
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