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

Adding recursive find on firstResponder UIView extension. #447

Merged
merged 13 commits into from Apr 19, 2018
Merged
4 changes: 3 additions & 1 deletion CHANGELOG.md
Expand Up @@ -6,13 +6,15 @@ The changelog for **SwifterSwift**. Also see the [releases](https://github.com/S
> ### Added
- **UITableViewExtentions**:
- Added `isValidIndexPath(_ indexPath:)` method to check whether given IndexPath is valid within UITableView. [#441](https://github.com/SwifterSwift/SwifterSwift/pull/441) by [setoelkahfi](https://github.com/setoelkahfi).
> ### Changed
### Changed
- **UITableViewExtentions**:
- `dequeueReusableCell(withClass:for)`, `dequeueReusableCell(withClass)` now return `UITableViewCell` object, `fatalError(...)` if not found. [#439](https://github.com/SwifterSwift/SwifterSwift/pull/439) by [jdisho](https://github.com/jdisho)
- `dequeueReusableHeaderFooterView(withClass)`now returns `UITableViewHeaderFooterView` object, `fatalError(...)` if not found. [#439](https://github.com/SwifterSwift/SwifterSwift/pull/439) by [jdisho](https://github.com/jdisho)
- **UICollectionView**:
- `dequeueReusableCell(withClass:for)` now return `UICollectionViewCell` object, `fatalError(...)` if not found. [#439](https://github.com/SwifterSwift/SwifterSwift/pull/439) by [jdisho](https://github.com/jdisho)
- `dequeueReusableSupplementaryView(ofKind:withClass:for)`now returns `UICollectionReusableView` object, `fatalError(...)` if not found. [#439](https://github.com/SwifterSwift/SwifterSwift/pull/439) by [jdisho](https://github.com/jdisho)
- **UIView**:
- `firstResponder` UIView extension now suport recursive find in the view hierarchy. [#447](https://github.com/SwifterSwift/SwifterSwift/pull/447) by [LucianoPAlmeida](https://github.com/LucianoPAlmeida).
> ### Deprecated
> ### Removed
> ### Fixed
Expand Down
19 changes: 14 additions & 5 deletions Sources/Extensions/UIKit/UIViewExtensions.swift
Expand Up @@ -86,11 +86,20 @@ public extension UIView {

/// SwifterSwift: First responder.
public var firstResponder: UIView? {
Copy link
Member

Choose a reason for hiding this comment

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

We may want to consider making this a function now and mention it's recursive behavior in the comment.
/// SwifterSwift: Recursively find the first responder.

guard !isFirstResponder else { return self }
for subView in subviews where subView.isFirstResponder {
return subView
}
return nil
guard
!isFirstResponder else { return self }
Copy link
Member

Choose a reason for hiding this comment

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

Not gonna lie, this style really irks me 😂 Nit though

Copy link
Member Author

Choose a reason for hiding this comment

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

😂

guard
!subviews.isEmpty else { return nil }

if let responder = subviews.first(where: { $0.isFirstResponder }) {
return responder
}
for subview in subviews {
Copy link
Member

Choose a reason for hiding this comment

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

This can be replaced with for subview in subviews where let responder = subview.firstResponder { ... }

Copy link
Member Author

Choose a reason for hiding this comment

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

@omaralbeik I've tried that, but was givin compilation error ¯_(ツ)_/¯
screen shot 2018-04-13 at 07 22 38

Copy link
Member

Choose a reason for hiding this comment

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

hmm, how about this

for subview in subviews where subview.firstResponder() != nil {
    return subview.firstResponder()!
}

Copy link
Member Author

Choose a reason for hiding this comment

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

@omaralbeik I think this is no good because it calls the recursive function twice, also see the "!" really irks me 😂

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I hate seeing force unwrapping in my code. The only time I do an explicit nil check and then force unwrap is if I don't care about the unwrapped value. If I'm going to use the value then I always use an if let.

if let responder = subview.firstResponder {
return responder
}
}
return nil
}

// SwifterSwift: Height of view.
Expand Down
24 changes: 22 additions & 2 deletions Tests/UIKitTests/UIViewExtensionsTests.swift
Expand Up @@ -42,8 +42,28 @@ final class UIViewExtensionsTests: XCTestCase {
}

func testFirstResponder() {
let view = UIView()
XCTAssertNil(view.firstResponder)
// When there's no firstResponder
XCTAssertNil(UIView().firstResponder)

let window = UIWindow()

// When self is firstResponder
let txtView = UITextField(frame: CGRect.zero)
window.addSubview(txtView)
txtView.becomeFirstResponder()
XCTAssertTrue(txtView.firstResponder === txtView)

// When a subview is firstResponder
let superView = UIView()
window.addSubview(superView)
let subView = UITextField(frame: CGRect.zero)
superView.addSubview(subView)
subView.becomeFirstResponder()
XCTAssertTrue(superView.firstResponder === subView)

// When you have to find recursively
XCTAssertTrue(window.firstResponder === subView)

}

func testHeight() {
Expand Down