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] Fixes issues with the arrangedSubviews property #17

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Thomvis
Copy link
Contributor

@Thomvis Thomvis commented Jun 25, 2015

This is quite a big PR, so please bear with me and let me know if you are open for changes like this.

I found that the arrangedSubviews property was not properly implemented. For starters, it was always nil. Looking for a solution, I found that the heavy reliance on self.subviews made it impossible to implement arranged subviews. I started shuffling things around and here we are...

Main improvements:

  • arrangedSubviews is now non-nil and should be kvo'able
  • removing an arranged view now does not remove the view as a subview
  • it should now be possible to have subviews in the OAStackView that are not managed by the stack view

All the tests pass and the example project runs fine, but I'd like to do some more testing in my own project. Let me know what you think!

- removing a view from the arranged subviews now does not remove it as
a subview (like UIStackView)
- this needed a strong separation between subviews and
arrangedSubviews, not just having arrangedSubviews be all non-hidden
subviews
- implemented the arrangedSubviews @Property, which was previously
unused
- arrangedSubviews is now KVO’able
@Thomvis
Copy link
Contributor Author

Thomvis commented Jun 25, 2015

I've discovered some bugs after I filed the PR, so it definitely deserves the [WIP] annotation. I will push additional changes, but in the meantime it would be great if you think this direction is favorable.

@nsomar
Copy link
Owner

nsomar commented Jun 25, 2015

This looks like a change to the correct direction, working with visible views was not really the optimal solution and I wanted to move away from it. I agree with your decision with going with arranged view storage.
I am really interested in seeing where this path leads 👍
Great work!

…in the aligning axis

This prevents arranged subviews from growing outside the stack view’s
frame. This could happen if the size of the stackview was constrained
so it could not become its intrinsic size.
@Thomvis
Copy link
Contributor Author

Thomvis commented Jun 27, 2015

I just pushed some more commits to this PR. Not all of them strictly relate to the arrangedSubviews change, but I've been trying to make improvements to OAStackView while integrating it into a project at work and some changes now depend on each other. ffcde3b for example has more to do with #14 (and #14 should not be merged without it). I'll explain the additional changes:

  • 3058581 the subviews of the stack need to shrink if the stack's size is constrained. The intrinsicContentSize based approach did not cover this. Instead, the alignment strategies now add additional constraints to make sure that the stack and subviews grow/shrink together.
  • ffcde3b this is needed for [WIP] Adding baseline align support #14 or else the baseline constraints will not be removed properly. I am not sure about the compiler checks for the newer NSLayoutAttributes. Let me know what you think.
  • fc97522 implements the UIStackView approach for baseline aligning a stack view itself. I'm not sure if this is as powerful as UIStackView, since we're probably missing some private API that will update the baseline alignment when the contents of the stack changes.

@Thomvis Thomvis mentioned this pull request Jun 27, 2015
3 tasks
@mthole-old
Copy link

@Thomvis — Ping on this one? I just ran into some of the arrangedSubviews you described here and am wondering what the state of this is. Thanks!

view.translatesAutoresizingMaskIntoConstraints = NO;
[self addSubview:view];
- (void)didAddSubview:(UIView *)subview {
[self addObserverForView:subview];

Choose a reason for hiding this comment

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

missing calls to super

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.

4 participants