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

BrickCell width provider #162

Merged
merged 1 commit into from
Jul 19, 2017
Merged

BrickCell width provider #162

merged 1 commit into from
Jul 19, 2017

Conversation

rubencagnie
Copy link
Contributor

On layoutSubviews, the cell will now verify if the width of the frame is equal to the expected width. If so, framesDidLayout will be called. This way, the brick can get informed that the frames are set correctly and images etc can be fetched.

Fixes #161

@rubencagnie rubencagnie force-pushed the 161-frames-did-update branch 2 times, most recently from 72430f8 to 5aed4fe Compare July 18, 2017 15:49
@@ -106,6 +111,15 @@ open class BaseBrickCell: UICollectionViewCell {
open override func layoutSubviews() {
super.layoutSubviews()
brickBackgroundView?.frame = self.bounds

if frame.width == requestedWidth {
self.layoutIfNeeded() // This layoutIfNeeded is added to make sure that the subviews are laid out correctly
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that called by super.layoutSubviews()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think I understand the question

Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine

Copy link
Contributor

Choose a reason for hiding this comment

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

Does calling layoutIfNeeded(), also call layoutSubviews()? or does the "if needed" mean only if self.bounds has changed? Just trying to make sure that there isn't a condition that this can get into an infinite loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It won't. The reason it's there is that when the view is laid out, the subviews aren't at this case. If we remove this line, the unit tests will fail

open override func framesDidLayout() {
super.framesDidLayout()

guard let genericContentView = genericContentView as? UpdateFramesListener else {
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be cleaner as an if let

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Fixing

On layoutSubviews, the cell will now verify if the width of the frame is equal to the expected width. If so, `framesDidLayout` will be called. This way, the brick can get informed that the frames are set correctly and images etc can be fetched.

Fixes #161
@codecov-io
Copy link

codecov-io commented Jul 19, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@460ba5f). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #162   +/-   ##
=========================================
  Coverage          ?   93.42%           
=========================================
  Files             ?       39           
  Lines             ?     4107           
  Branches          ?      333           
=========================================
  Hits              ?     3837           
  Misses            ?      269           
  Partials          ?        1
Impacted Files Coverage Δ
Source/Cells/BrickCell.swift 98.48% <100%> (ø)
Source/Bricks/Generic/GenericBrick.swift 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 460ba5f...d7cfc31. Read the comment docs.

@jay18001 jay18001 merged commit deb2cb3 into master Jul 19, 2017
@jay18001 jay18001 deleted the 161-frames-did-update branch July 19, 2017 15:04
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

4 participants