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

Weird conditional #145

Closed
davertay opened this issue Feb 14, 2018 · 4 comments
Closed

Weird conditional #145

davertay opened this issue Feb 14, 2018 · 4 comments

Comments

@davertay
Copy link

davertay commented Feb 14, 2018

Is this a typo where the second condition is meant to be height instead of width?
https://github.com/ashfurrow/Nimble-Snapshots/blob/master/DynamicSize/DynamicSizeSnapshot.swift#L71

Updating this with a secondary query. A few lines before this layoutIfNeeded() is called, however as setNeedsLayout() is not called first it is almost guaranteed that the conditional view.bounds.width != size.width will fail.

So then my question really changes to: can this entire "iOS 9+ BUG" hack be avoided by tripping the setNeedsLayout() flag instead?

@ashfurrow
Copy link
Owner

Hmm, good question. I think it is a typo, but I'm not sure about the rest. @marcelofabri what do you think?

@marcelofabri
Copy link
Collaborator

This looks weird in fact. I think it should be height. Maybe @BrunoMazzo can give some insights for the whole "iOS 9+ BUG" section too as he was the original author?

@BrunoMazzo
Copy link
Contributor

It's appear to be really a typo. Please change it to height.

But I do believe that adding setNeedsLayout() and removing the condition will break the feature for iOS 9. I tried here, it's simple, just run all tests on a iPhone 6 iOS 9.3 to see. The bug is that on iOS 9 auto-layout do not work on views that don't have a superview or a window (I don't remember exactly). I don't now why and don't have the link where a found about it.

But feel free to change anything! Just run the test on iOS 9, if it pass it's OK. If you find another solution I will be very glad (I really don't like this bug fix, it's a ugly if, 😢).

davertay pushed a commit to davertay/Nimble-Snapshots that referenced this issue Mar 12, 2018
ashfurrow added a commit that referenced this issue Mar 16, 2018
Issue #145 fixed check to use both height and width instead of just width
@Vkt0r
Copy link
Collaborator

Vkt0r commented Mar 30, 2018

I think we can close this as #145 solved

@Vkt0r Vkt0r closed this as completed Mar 30, 2018
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

No branches or pull requests

5 participants