add findElementLocator

[erikolsson]

Hi!

Here's a rough implementation of what findElementLocator(at point: CGPoint) could look like. Not sure if there's a cleaner way to do the coordinate transformation in EPUBSpreadView.swift?

[mickael-menu]

Thank you, this will be a very useful addition!

[mickael-menu]

Would you mind refactoring to avoid the code duplication with findFirstVisibleLocator? Maybe adding a locatorToElement() function.

[mickael-menu]

I suggest renaming elementLocator() to just locator(). In firstVisibleElementLocator, the "element" part was related to "first visible element".

Suggested change

	/// Returns the `Locator` to the first content element located at the given point
	func elementLocator(at point: CGPoint, completion: @escaping (Locator?) -> Void)
	/// Returns the `Locator` to the content visible at the given view coordinates.
	func locator(at point: CGPoint, completion: @escaping (Locator?) -> Void)

[mickael-menu]

I realize you reused the default logic from firstVisibleElementLocator, but I wonder if it wouldn't be better to return nil if it's not implemented in a navigator, for this API?

When an app calls firstVisibleElementLocator(), it's to get more or less to the top of the page, so currentLocation is alright (although maybe it should have been nil too). But when you intend to get a Locator to a specific point, it might be confusing if you get the top of the page.

Apps can still fallback on currentLocation manually when receiving nil.

[mickael-menu]

This is useful stuff but also easy to get it wrong, depending on the iOS version or FXL vs reflowable layouts.

Maybe we could add an equivalent to these two APIs but in the other direction (fromNavigatorSpace)?

swift-toolkit/Sources/Navigator/EPUB/EPUBSpreadView.swift

Lines 216 to 226 in 4f082ee

	/// Converts the given JavaScript point into a point in the webview's coordinate space.
	func convertPointToNavigatorSpace(_ point: CGPoint) -> CGPoint {
	// To override in subclasses.
	return point
	}
	/// Converts the given JavaScript rect into a rect in the webview's coordinate space.
	func convertRectToNavigatorSpace(_ rect: CGRect) -> CGRect {
	// To override in subclasses.
	return rect
	}

For inspiration, here's the reflowable implementation:

swift-toolkit/Sources/Navigator/EPUB/EPUBReflowableSpreadView.swift

Lines 133 to 158 in 4f082ee

	override func convertPointToNavigatorSpace(_ point: CGPoint) -> CGPoint {
	var point = point
	if isScrollEnabled {
	// Starting from iOS 12, the contentInset are not taken into account in the JS touch event.
	if #available(iOS 12.0, *) {
	if scrollView.contentOffset.x < 0 {
	point.x += abs(scrollView.contentOffset.x)
	}
	if scrollView.contentOffset.y < 0 {
	point.y += abs(scrollView.contentOffset.y)
	}
	} else {
	point.x += scrollView.contentInset.left
	point.y += scrollView.contentInset.top
	}
	}
	point.x += webView.frame.minX
	point.y += webView.frame.minY
	return point
	}
	override func convertRectToNavigatorSpace(_ rect: CGRect) -> CGRect {
	var rect = rect
	rect.origin = convertPointToNavigatorSpace(rect.origin)
	return rect
	}

and the FXL implementation:

swift-toolkit/Sources/Navigator/EPUB/EPUBFixedSpreadView.swift

Lines 100 to 115 in 4f082ee

	override func convertPointToNavigatorSpace(_ point: CGPoint) -> CGPoint {
	CGPoint(
	x: point.x * scrollView.zoomScale - scrollView.contentOffset.x + webView.frame.minX,
	y: point.y * scrollView.zoomScale - scrollView.contentOffset.y + webView.frame.minY
	)
	}
	override func convertRectToNavigatorSpace(_ rect: CGRect) -> CGRect {
	var rect = rect
	rect.origin = convertPointToNavigatorSpace(rect.origin)
	rect.size = CGSize(
	width: rect.width * scrollView.zoomScale,
	height: rect.height * scrollView.zoomScale
	)
	return rect
	}

[mickael-menu]

Hi @erikolsson, are you still interested in getting this PR merged?

[erikolsson]

Hi @mickael-menu!
I'm terribly sorry, I took another approach (wrote my own renderer in UIKit) and ran out of time! I will probably not have time to do these changes in the near future, but if someone else has the time, please feel free to make whatever changes you see fit to this PR!

[mickael-menu]

Alright, thanks for the heads-up. I might take over the PR once I have some leeway.