jessesquires/PresenterKit

Popover sourceRect should be set to view's bounds rather than frame?

Closed this issue · 4 comments

Recently I experienced strange behaviour with popovers presented on iPad: I tried to present a popover and it didn't appear on screen. No exception was thrown and UI became kind of frozen for a few moments/taps. After that it worked exactly as it would worked if the popover was cancelled.

My application's UI is quite complex and I don't have time to reproduce this case in a small project. The problem happened when I presented a popover from a UITableViewCell located at the end of a long UITableView. User had to scroll to reach the presenting cell. When tapped a popover is presented asking confirmation. My view's frame is something like:

▿ (0.0, 982.0, 768.0, 53.0)
  ▿ origin : (0.0, 982.0)
    - x : 0.0
    - y : 982.0
  ▿ size : (768.0, 53.0)
    - width : 768.0
    - height : 53.0

And its bounds are:

▿ (0.0, 0.0, 768.0, 53.0)
  ▿ origin : (0.0, 0.0)
    - x : 0.0
    - y : 0.0
  ▿ size : (768.0, 53.0)
    - width : 768.0
    - height : 53.0

(As expected. Frame are view's coordinates expressed in parent's view coordinate system and bounds are view's coordinates expressed in view's coordinate system, so bounds are expected to have a 0 origin and frame's size).

Checking what does PresenterKit I've confirmed that it currently sets view's frame as popover's sourceRect: https://github.com/jessesquires/PresenterKit/blob/develop/Source/ViewControllerExtensions.swift#L129

However checking documentation on UIPopoverPresentationController:

var sourceView: UIView?
The view containing the anchor rectangle for the popover.
var sourceRect: CGRect
The rectangle in the specified view in which to anchor the popover.

Which made me wonder whether PresenterKit should be setting view's bounds as sourceRect instead of frame.

I've tried changing that line and it fixed my issues but without proper test cases or confirmation by a fellow developers I'm not really sure whether this change is actually fixing the issue or just hiding the symptom.

Thanks @Sumolari ! Great report. 💯

So PresenterKit is doing this correctly according to the docs:

var sourceRect: CGRect { get set }

Use this property in conjunction with the sourceView property to specify the anchor location for the popover.

In other words:

  • sourceView == the view containing the popover
  • sourceRect == the anchor rect inside sourceView

This means we definitely do not want .bounds here. If we did that, then every popover would be showing in the top left corner.


I'm not really sure whether this change is actually fixing the issue or just hiding the symptom.

Sounds to me like this change is hiding a symptom 😄

In other words:

  • sourceView == the view containing the popover
  • sourceRect == the anchor rect inside sourceView

Are you sure about that? Checking the docs:

var sourceView: UIView?
The view containing the anchor rectangle for the popover.
var sourceRect: CGRect
The rectangle in the specified view in which to anchor the popover.

It seems that sourceView does not contain the popover but the rectangle in which popover is anchored. That also matches my experience: I've presented popovers using as sourceView smaller views like buttons or table view cells.

I've also been able to create a small test project and recorded a video of the behaviour. When I use as sourceView my instance of UITableViewCell popover isn't anchored properly, however when I use as sourceView its contentView it's properly anchored. Both views have almost the same size but different origin:

  • Cell:
    • frame: (0.0, 308.0, 768.0, 44.0)
    • bounds: (0.0, 0.0, 768.0, 44.0)
  • contentView:
    • frame: (0.0, 0.0, 768.0, 43.5)
    • bounds: (0.0, 0.0, 768.0, 43.5)

Hi all 😀,

As we can see from the docs the sourceView and sourceRect are somehow playing together. So I had a play with different scenarios in order to try to find an explanation regarding this issue.
Firstly if we ignore the sourceRect the popover is still pointing to the right cell but the alignment is not correct..
screen shot 2017-01-31 at 10 40 34
So as we can understand in this instance the popover used some information from the sourceView and most probably the sourceView.frame since at least the origin of the pointing arrow seems correct. I can only assume based on that, that the reason it can work with the .bounds is because let's say the popover respects the origin of the sourceView and the size of the sourceRect (maybe the reason that ignores the origin of the sourceRect is because it is .zero) .
The conclusion on the first of the experiments is that we most probably we should provide a way for passing a custom sourceRect and not take for the granted the frame of the sourceView.
That leads me to the solution that I found working. First of all I quickly introduced a way of passing a customRect and I did the following:
instead of var config = PopoverConfig(source: .view(cell)) I pass the tableView var config = PopoverConfig(source: .view(tableView)) which I believe it can make sense since

sourceView == the view containing the popover

and then all I had to do is to pass the frame of the cell , config. customRect = cell.frame (which means popoverController?.sourceRect = c. customRect)
screen shot 2017-01-31 at 10 54 17

So to sum up, as I said before we should probably introduce a way of passing a customRect (maybe in a different way that I did) 😀

Nice! Thanks @Sumolari and @psartzetakis ! 💯

Proposed solution:

Update PopoverConfig.Source.view to include an optional frame.

Old:

case view(UIView)

New:

case view(container: UIView, frame: CGRect?)

If frame is nil, then we'll use container.frame.