Fix wrap content calculation by filter hidden views #280

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

Jump to bottom

Open

ruslandzhafarov wants to merge 10 commits into layoutBox:master

from ruslandzhafarov:patch-1

Open

Fix wrap content calculation by filter hidden views #280

ruslandzhafarov wants to merge 10 commits into layoutBox:master from ruslandzhafarov:patch-1

Conversation

@ruslandzhafarov

Copy link

@ruslandzhafarov ruslandzhafarov commented Mar 28, 2024

No description provided.

@ruslandzhafarov


 Fix wrap content calculation by filter hidden views

e20a081

@ruslandzhafarov ruslandzhafarov closed this

Mar 28, 2024

@ruslandzhafarov ruslandzhafarov deleted the patch-1 branch

March 28, 2024 16:55

@ruslandzhafarov ruslandzhafarov restored the patch-1 branch

March 28, 2024 16:56

lucdion

lucdion requested changes

Mar 28, 2024

View reviewed changes

Sources/PinLayout+WrapContent.swift Outdated Show resolved Hide resolved

@ruslandzhafarov ruslandzhafarov reopened this

Mar 28, 2024

Ruslan Dzhafarov added 3 commits

March 28, 2024 19:41


 Fix wrap content calculation by filter hidden views

98258f8


 Add comments

fb3a375


 minor fixes

7d756bc

@ruslandzhafarov ruslandzhafarov requested a review from lucdion

March 28, 2024 18:22

MontakOleg

MontakOleg reviewed

Mar 28, 2024

View reviewed changes

Sources/PinLayout+WrapContent.swift Outdated

let subviews: [PinView.PinView]

switch viewFilter {

case .visibleOnly:

subviews = view.subviews.filter { !0ドル.isHidden }

Copy link

Contributor

@MontakOleg MontakOleg Mar 28, 2024

Choose a reason for hiding this comment

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

What about views with alpha = 0?

There is public visible(_ views: [UIView]) views filter that takes alpha into account -

PinLayout/Sources/Filters.swift

Line 25 in babea9c

return views.filter({ !0ドル.isHidden && 0ドル.alpha > 0 })

, it may lead to confusion if name "view filter" will behave differently here.

Copy link

Member

@lucdion lucdion Mar 28, 2024

Choose a reason for hiding this comment

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

Good point, you could use the visible() here to filter views.

MontakOleg

MontakOleg reviewed

Mar 28, 2024

View reviewed changes

Sources/PinLayout+WrapContent.swift Outdated

private func wrapContent(_ type: WrapType, padding: PEdgeInsets, _ context: Context) -> PinLayout {

let subviews = view.subviews

private func wrapContent(_ type: WrapType, padding: PEdgeInsets, viewFilter: ViewFilter = .none, _ context: Context) -> PinLayout {

Copy link

Contributor

@MontakOleg MontakOleg Mar 28, 2024

Choose a reason for hiding this comment

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

do we need default argument value (viewFilter = .none) in private function?

Copy link

Member

@lucdion lucdion Mar 28, 2024

Choose a reason for hiding this comment

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

Not needed here, the parameter should always be specified by callers

lucdion

lucdion reviewed

Mar 28, 2024

View reviewed changes

Copy link

Member

@lucdion lucdion left a comment

Choose a reason for hiding this comment

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

Excellent. Could you update the documentation of wrapContent (README.md)? 🙏

Sources/PinLayout+WrapContent.swift Outdated

let subviews: [PinView.PinView]

switch viewFilter {

case .visibleOnly:

subviews = view.subviews.filter { !0ドル.isHidden }

Copy link

Member

@lucdion lucdion Mar 28, 2024

Choose a reason for hiding this comment

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

Good point, you could use the visible() here to filter views.

Sources/Types.swift Outdated Show resolved Hide resolved


 minor fixes

6d81204

MontakOleg

MontakOleg reviewed

Mar 28, 2024

View reviewed changes

Sources/Layoutable.swift Outdated Show resolved Hide resolved

Ruslan Dzhafarov added 2 commits

March 29, 2024 00:54


 minor fixes

69903d5


 update README.md

46e93fa

@ruslandzhafarov ruslandzhafarov requested a review from lucdion

March 28, 2024 23:07

lucdion

lucdion reviewed

Apr 5, 2024

View reviewed changes

Sources/Extensions/CALayer+PinLayout.swift Outdated Show resolved Hide resolved

Sources/Extensions/NSView+PinLayout.swift Outdated

}

public var isVisible: Bool {

!isHidden && alphaValue > 0

Copy link

Member

@lucdion lucdion Apr 5, 2024

Choose a reason for hiding this comment

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

Suggested change

!isHidden && alphaValue > 0

return !isHidden && alphaValue > 0

Sources/Extensions/UIView+PinLayout.swift Outdated

}

public var isVisible: Bool {

!isHidden && alpha > 0

Copy link

Member

@lucdion lucdion Apr 5, 2024

Choose a reason for hiding this comment

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

Suggested change

!isHidden && alpha > 0

return !isHidden && alpha > 0

Sources/Types.swift Outdated Show resolved Hide resolved

Sources/Layoutable.swift Outdated Show resolved Hide resolved

Ruslan Dzhafarov added 2 commits

April 5, 2024 20:23


 minor fixes

44697d4


 update README.md

de0868e

@ruslandzhafarov ruslandzhafarov requested review from MontakOleg and lucdion

April 5, 2024 17:32

@winnisx7

Copy link

winnisx7 commented Dec 17, 2024

bump
why not merge?

lucdion

lucdion approved these changes

Dec 17, 2024

View reviewed changes

@lucdion


 Update README.md

1bcd966

Force rebuild

Labels

None yet

4 participants

@ruslandzhafarov @winnisx7 @MontakOleg @lucdion

Fix wrap content calculation by filter hidden views #280

Are you sure you want to change the base?

Fix wrap content calculation by filter hidden views #280

Uh oh!

Conversation

@ruslandzhafarov ruslandzhafarov commented Mar 28, 2024

Uh oh!

Uh oh!

@MontakOleg MontakOleg Mar 28, 2024

Choose a reason for hiding this comment

Uh oh!

@lucdion lucdion Mar 28, 2024

Choose a reason for hiding this comment

Uh oh!

@MontakOleg MontakOleg Mar 28, 2024

Choose a reason for hiding this comment

Uh oh!

@lucdion lucdion Mar 28, 2024

Choose a reason for hiding this comment

Uh oh!

@lucdion lucdion left a comment

Choose a reason for hiding this comment

Uh oh!

@lucdion lucdion Mar 28, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

@lucdion lucdion Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

@lucdion lucdion Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

winnisx7 commented Dec 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants