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 popoversourceRect
== the anchor rect insidesourceView
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)
- frame:
contentView
:- frame:
(0.0, 0.0, 768.0, 43.5)
- bounds:
(0.0, 0.0, 768.0, 43.5)
- frame:
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..
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
)
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
.