So I think this should be solved by passing a new property "maxCharWidth" to the instructionRow widget. Then if that property is set we pass it to the label and make sure it ellipsize when passing that width. That can be done with the ellipsize property on GtkLabel: https://developer.gnome.org/gtk3/stable/GtkLabel.html#GtkLabel--ellipsize

I wanted to look into this, but I'm not 100% sure I understand the issue. Do we want instruction labels to be ellipsized so they do not occupy more than 2 lines in the printed route?

It seems that instructionLabel in the instructionRow widget already has these properties:

<property name="ellipsize">end</property>
<property name="width_chars">20</property>
<property name="max_width_chars">20</property>
<property name="lines">3</property>

So anything larger than 3 lines actually gets ellipsized. I might have understood the issue wrongly, but shouldn't it be enough for us to overwrite the 'lines' property (and set it to 2) when creating InstructionRows from within printLayout?

(In reply to Razvan Brinzea from comment #2)
> I wanted to look into this, but I'm not 100% sure I understand the issue. Do
> we want instruction labels to be ellipsized so they do not occupy more than
> 2 lines in the printed route?
> 
> It seems that instructionLabel in the instructionRow widget already has
> these properties:
> 
> <property name="ellipsize">end</property>
> <property name="width_chars">20</property>
> <property name="max_width_chars">20</property>
> <property name="lines">3</property>
> 
> So anything larger than 3 lines actually gets ellipsized. I might have
> understood the issue wrongly, but shouldn't it be enough for us to overwrite
> the 'lines' property (and set it to 2) when creating InstructionRows from
> within printLayout?

Right. That should probably work. I think I would prefer setting it to 1 though.

(In reply to Jonas Danielsson from comment #3)
> (In reply to Razvan Brinzea from comment #2)
> > I wanted to look into this, but I'm not 100% sure I understand the issue. Do
> > we want instruction labels to be ellipsized so they do not occupy more than
> > 2 lines in the printed route?
> > 
> > It seems that instructionLabel in the instructionRow widget already has
> > these properties:
> > 
> > <property name="ellipsize">end</property>
> > <property name="width_chars">20</property>
> > <property name="max_width_chars">20</property>
> > <property name="lines">3</property>
> > 
> > So anything larger than 3 lines actually gets ellipsized. I might have
> > understood the issue wrongly, but shouldn't it be enough for us to overwrite
> > the 'lines' property (and set it to 2) when creating InstructionRows from
> > within printLayout?
> 
> Right. That should probably work. I think I would prefer setting it to 1
> though.

Yeah, that will work. Jonas i think setting it to 2 is a good idea, because it won't create issue of overlapping, plus it will be more informatory to user. Line 1 is almost covered up in displaying the direction. Road info gets more clearer in second line ,as far i have observed.

Created attachment 322183 [details]
Comparison between 1 and 2 lines per instruction

This is a comparison between having one line per instruction row and having two lines per instruction row, for a sample route I usually use to test. I agree with Amisha, I think having just one line cuts off too much information.

Created attachment 322184 [details] [review]
Ellipsize route instructions in print layout

This is the solution I've found. 'lines' is now an optional property that can be passed to the constructor of an InstructionRow, and it overrides the default setting of 3 lines that the InstructionRow labels have.

Review of attachment 322184 [details] [review]:

Thanks!

My one concern with this is that we have a fixed height for instructionRow that we use in calculations.
Could you check with the inspector (GTK_DEBUG=insteractive gnome-maps) that the allocated height for a row with 1 lines and one with 2 lines is the same?

::: src/instructionRow.js
@@ +35,3 @@
     _init: function(params) {
         this.turnPoint = params.turnPoint;
+

extra new line

@@ +41,3 @@
+            this.lines = params.lines;
+            delete params.lines;
+        }

pls follow the same style we use elsewhere, this becomes:

this._lines = params.lines;
delete params.lines;

No need to check if it there this._lines becomes undefined if not there, and that is ok.
And we use _ to prefix internal variables.

This can even be:

this._lines = params.lines || 1;

And then you do not need an if later, right?

@@ +47,3 @@
+        if('lines' in this) {
+            this._instructionLabel.lines = this.lines;
+        }

Then this becomes just:

this._instructionLabel.lines = this._lines;

(In reply to Razvan Brinzea from comment #5)
> Created attachment 322183 [details]
> Comparison between 1 and 2 lines per instruction
> 
> This is a comparison between having one line per instruction row and having
> two lines per instruction row, for a sample route I usually use to test. I
> agree with Amisha, I think having just one line cuts off too much
> information.

Thanks for this!

Review of attachment 322184 [details] [review]:

Thanks Razvan.

::: src/instructionRow.js
@@ +41,3 @@
+            this.lines = params.lines;
+            delete params.lines;
+        }

Jonas, won't using params.lines||1 override the lines property in other cases too ,other than printing purpose?
i think it should be just: 
this._lines = params.lines;
delete params.lines;

if (this._lines)
    this._instructionLabel.lines = this._lines;

Review of attachment 322184 [details] [review]:

::: src/instructionRow.js
@@ +41,3 @@
+            this.lines = params.lines;
+            delete params.lines;
+        }

Sure I thought the default value was 1 and then we could avoid an if-statement but you are right!

Created attachment 322282 [details] [review]
Ellipsize route instructions in print layout

I cleaned up the code.
Also, I used the inspector to check the allocated size of InstructionRows, and in both cases (1 line and 2 lines per row), the allocated size is 328x48, so using 2 lines should not cause any problems.

Also, I just noticed I slipped another newline, sorry about that. I seem to have missed it when using git diff.

Review of attachment 322282 [details] [review]:

Thanks Razvan for the patch. Looks good enough. just little nits.

::: src/instructionRow.js
@@ +42,3 @@
         this.parent(params);
 
+        if(this._lines) {

Styling of if loop is as follows:
if (this._lines) {

::: src/printLayout.js
@@ +213,3 @@
             this._addSurface(surface, x, y, pageNum);
         }).bind(this));
+

extra line

Review of attachment 322282 [details] [review]:

Thanks!

::: src/instructionRow.js
@@ +37,3 @@
         delete params.turnPoint;
 
+        this._lines = params.lines;

maybe just:

    let lines = params.lines?

I do not see that we need to have this variable as a member variable?

Created attachment 322829 [details] [review]
Ellipsize route instructions in print layout

Route instructions are ellipsized after two lines
in order to prevent overlapping in the printed
route.

Review of attachment 322829 [details] [review]:

Thanks!

I will commit this with some changes. But, please in the future make sure the patch is against _master_, this did not apply cleanly and I had to fix it up.

Review of attachment 322829 [details] [review]:

pushed