Created attachment 232052 [details] [review]
boxpointer: Clean up reposition

Given that it's modifying state, there's no reason it can't be
using it instead. This makes it easier to use, so we aren't passing
in a bunch of instance variables every time.

Created attachment 232053 [details] [review]
boxpointer: Update flip on setPosition as well as allocate

This is needed for popup menus like ShellEntry and the soon-to-be-introduced
BackgroundMenu to flip automatically correctly.

Created attachment 232054 [details] [review]
boxpointer: Rework how flipping works

Make sure we re-allocate after we flip sides, to ensure that
padding around the child actor is updated correctly.

Created attachment 232055 [details] [review]
boxpointer: Don't use the box allocation when calculating the arrow side

Depending on the current state of arrowSide, the box allocation may be
wrong; e.g. if the user requested a TOP, but we flipped to a BOTTOM, the
next request would look to the y2 value of the flipped BOTTOM, which is
wrong.

Instead, use the origin, plus the calculated preferred size of the box.

Created attachment 232056 [details] [review]
boxpointer: Defer re-allocation after a flip

As we may be flipping the box pointer in response to re-allocation,
like the addition of a new actor to the boxpointer, we can't queue
a re-layout while in a re-layout, so defer.

Review of attachment 232052 [details] [review]:

sure

Review of attachment 232056 [details] [review]:

Ok

Review of attachment 232055 [details] [review]:

Looks good.

Review of attachment 232054 [details] [review]:

::: js/ui/boxpointer.js
@@ -524,3 @@
         this._yPosition = Math.floor(y);
         this._shiftActor();
-        this._updateFlip();

This got added here on the previous patch and is removed now - correctly, I think, so I'd squash these two patches.

Review of attachment 232053 [details] [review]:

::: js/ui/boxpointer.js
@@ +524,3 @@
         this._yPosition = Math.floor(y);
         this._shiftActor();
+        this._updateFlip();

Potentially an endless loop here.

@@ +597,3 @@
+                break;
+            }
+            this._reposition(this._sourceActor, this._arrowAlignment);

Arguments - something funny with the patch ordering?

Created attachment 232065 [details] [review]
boxpointer: Rework how flipping works

Make sure we re-allocate after we flip sides, to ensure that
padding around the child actor is updated correctly. Additionally,
ensure that we flip after we setPosition, as we won't get re-allocated
auotmatically by just changing the position.



Merge the two? Something like this?

Review of attachment 232065 [details] [review]:

Yes.

Attachment 232052 [details] pushed as b14b3ab - boxpointer: Clean up reposition
Attachment 232055 [details] pushed as d6cace3 - boxpointer: Don't use the box allocation when calculating the arrow side
Attachment 232056 [details] pushed as 43876a9 - boxpointer: Defer re-allocation after a flip
Attachment 232065 [details] pushed as 2388de4 - boxpointer: Rework how flipping works