Possible improvement to the layout engine (of which the code is a bit confusing)

[roylly]

Original Title: computeSegmentIndex to layout coachMarkView horizontally make me confused

As i want to make the arrow orientation to support left and right，i read the source code seriously. The complex layout code make me confused, why " Compute the segment index (for now the screen is separated in three horizontal areas and depending in which one the coach mark stand, it will be layed out in a different way."? why not layout the coachMarkView around the cutout path frame and calculate the maxWidth of coachMarkView? I think it will make layout code easy to understand.

[ephread]

One word : Legacy 😄

The layout engine was originally written in Objective-C and supported iOS 7. At the time I came across some significant hurdles† with Auto Layout and I chose a (conceptually simple) segment system instead of doing a multi-pass layout. It's definitely due for a revamp and I will tackle the issue soon.

† I no longer remember clearly what these hurdles were. Auto Layout had some messiness at the beginning and my knowledge of AL was also much more limited in 2013.

---

Okay, I lied a bit, it's not just about Legacy. I'm going to use this issue as an opportunity to lay out my thoughts on the matter (I've updated the title of the issue to reflect that). This comment is going to be a bit long and also related to #160 and #152. Hopefully it'll enable you or others to point out if I overcomplicate the problem or if I'm missing key elements.

Before I start, let me define the term axis. In the current context there are two possible axes (Top-Bottom and Leading-Trailing) for laying out coach marks. Instructions, at the moment, only allow one axis (Top-Bottom) that means that coach marks can neither be on the left of a cutout path nor on the right, only above, or below.

Let's get to the meat of it.

why not layout the coachMarkView around the cutout path frame and calculate the maxWidth of coachMarkView?

Positioning coach marks a bit trickier than simply centering the coach mark around the cutout path (no matter the axis), there are certain edge cases which need to be taken into account. For instance, if the cutout path is close to the edges of the screen, but the content of the coach mark is significant, you'll either end up with nearly half of the coach mark outside of the screen bounds or with a tiny width and significant word/character wrapping, making the content barely decipherable (depending on which constraint system you choose). These edge cases exist no matter how many axes you allow (top-bottom / left-right).

And, although scarcely seen in practice, coach marks can also contains very complex layouts, not just a single label with intrinsic size. That had to be taken into account.

That's more or less why I came up with this basic system. The reasoning being that it should behave a bit like text alignment. If the cutout is in the first third of the screen (in a LTR environment), then the body will have a fixed leading constraint and a dynamic trailing constraint (vice versa for the third third). If it's in the second third, then we would center it.

This isn't a solution that I'm fully happy about, though. I'd like to improve the engine iteratively and use multiple passes.

Improvement 1

The first improvement would be to do away with the discreet segment system and go for a continuous one, but this would require a two-passes system, as I don't believe it's possible by only using Auto Layout constraints.

It think it could follow the following sequence:

1. Render the coach mark body offscreen, and check its actual width (<= CoachMark.maxWidth)
2. Isolate the horizontal coordinates (yVal) of the point of interest and check if the rendered coach mark (centered on yVal) would end up being partially offscreen.
3. If that's the case, set-up constraints accordingly to make sure that the coach mark is entirely visible (using the current code).

Obvioulsy, that wouldn't remove most of the code generating the constraints. But it would fix #160.

Improvement 2

At the moment, the engine chooses to put the coach mark above or below based on a very simple check of pointOfInterest.y. If it’s above half the screen, then the coach mark will be displayed below. If it’s below, then it’ll be displayed above. Again, a multi-pass system would greatly benefit the engine, since we would now be able to choose where to place the coach mark based on its size and the remaining space available around the cutout path.

Combined with the ability to center the coach mark (without arrow) on the cutout path, it would fix #152.

---

Once these two improvement are made, maybe we could make the code axis-agnostic, set a preferred axis and if it doesn't fit within this axis, try the other one.

In addition to these improvements, I've always wanted Instructions to provide smart defaults. Meaning that if you don’t override anything, it should put the coach mark at the best position possible (it doesn't do that very well for edge cases at the moment). For that to happen, I originally left out the ability to position the coach marks on either side of the cutout and focused on top/bottom positioning, because I felt that the little benefits that it would provide did not outweight the increased complexity.

I'm more than happy to discuss the topic, however. How would you go about rethinking the engine?

[roylly]

wow, so much information. I want to expand some function for Instruments. and i will try to code with your design thought.

[roylly]

hi, i have finished the arrow orientation support left and right development, but don't make big change for the layout engine, just add new case for layout. And the arrowView isn't a UIImageView anymore, but a UIView,and draw the arrow according the orientation, so the “highlighted” not work temporarily(i think that's not hard to fix). While , i also add backgroundImageView var for interface of BodyView, then user can hidden it,and set custom background color if they want.
https://github.com/roylly/Instructions
do you have some advice?

[ephread]

I've quickly glanced at your fork, it looks great, thanks for working on this!

I do nonetheless have a number of comment to make about the change, can you make a PR to the layout-engine branch that I just created, so we can discuss the changes there more easily?

On a side note, I'm going to write a CONTRIBUTING.md –which I never bothered to write before. It will make things easier, especially style-wise.

[roylly]

ok, i have made a PR.