[PATCH] optimize the space in the history view (column width) and consider adding line wrapping

[dieterdreist]

Currently I have a hard time with long values in the history view, because I have to hover over the window, often also bring it to focus, or adjust the central column width every time.

I think the column width of the first and last column could be optimized to reduce to the minimum possible width without having text overflow and give this width to the column which hasn't sufficient space to show the whole value. In particular the "since" column, showing usually only two digits, it way too wide. If this isn't enough, consider wrapping the field content over several rows. Additionally or alternatively, it could remember manually adjusted column widths.

Here is how it currently looks like:

[dieterdreist]

current default state of history view

[dieterdreist]

Ticket #23483 has been marked as a duplicate of this ticket.

[taylor.smock]

I think we just have to set a max width for Since, like so:

src/org/openstreetmap/josm/gui/history/TagTableColumnModel.java

@@ -41 +42 @@
         col.setHeaderValue(tr("Since"));
         col.setCellRenderer(renderer);
         col.setPreferredWidth(10);
+        col.setMaxWidth(new JLabel("v" + Integer.MAX_VALUE).getWidth());
         addColumn(col);
     }
 }

I'd apply it, but I'm having issues with the OSM API right now.

I also don't know what the "maximum" version for an object is. Integer.MAX_VALUE (2147483647) is something that we'll probably never see.

[GerdP]

Doesn't work for me, new JLabel("v" + Integer.MAX_VALUE).getWidth() returns 0.

[taylor.smock]

This should work better:

src/org/openstreetmap/josm/gui/history/TagTableColumnModel.java

@@ -40 +41 @@
         col = new TableColumn(COLUMN_VERSION);
         col.setHeaderValue(tr("Since"));
         col.setCellRenderer(renderer);
-        col.setPreferredWidth(10);
+        /* "." padding to avoid Since being cut off to be Sin... */
+        col.setPreferredWidth(new JLabel(tr("Since") + ".").getMinimumSize().width);
+        col.setMaxWidth(new JLabel("v" + Long.MAX_VALUE).getMinimumSize().width);
         addColumn(col);
     }
 }

[GerdP]

default window size

[GerdP]

enlarged window

[GerdP]

patched original window size

[GerdP]

patched enlarged window

[GerdP]

sorry for the late response. I think the original screen shot shows the history for relation r7317843.

I think the patch is an improvement. I've attached four screen shots created with a clean home dir and latest version r19007
With the original window size the difference is minimal, but the enlarged version shows the improvement.

That said the code probably doesn't work as expected as the header of the "Since" (or "Seit" in my case) column is now always truncated to "..." if the version numbers are small. I think this is because the calculated values for setPreferredWidth (22 in my case) and setMaxWidth (120) are just used as hints for the calculation of the real column widths. Since we use hardcoded value 100 for the other two columns this cannot work well.

I think the only good way to fix this is to analyse the actual data in the history:
Find the largest values for the rendered columns and the use those to set the widths.

Edit: I wonder what the longest translation for "Since" would be and if we really want to always show that untruncated.

[dieterdreist]

This is a huge improvement.

For the since column: maybe there could just be an asterisk in the column head, with an explanation "* = since" somewhere else?

I think wrapping long rows would also be benefitial, truncating the content makes it very hard to compare different values.

[taylor.smock]

patched original window size (mac)

[taylor.smock]

patched enlarged window (mac)

[GerdP]

Did you try the mentioned relation? It only has version up to v9.

[taylor.smock]

I hate it when there is different behavior on different operating systems. Although it might be a locale issue.

@dieterdreist: fiddling with the Since column is a lot easier since we know what the absolute maximum size is.

Did you try the mentioned relation? It only has version up to v9.

Yes. It did work properly for me.

[GerdP]

Your dialog is quite different. Some texts are centered where on Windows they are left aligned.

[taylor.smock]

I wonder what the longest translation for "Since" would be and if we really want to always show that untruncated.

The longest I've found (on google translate) is Samoan, "Talu mai lena taimi" (roughly "since then"). There is probably a better translation, but google translate doesn't know it. Anyway, that gives us a "worst case" length.

EDIT: Moving the preferred width after setting the max width might work, if we do a Math.min call.

        col.setPreferredWidth(Math.min(new JLabel(tr("Since") + ".").getMinimumSize().width,
                col.getMaxWidth()));

Your dialog is quite different. Some texts are centered where on Windows they are left aligned.

FML. This is probably a Windows LaF difference. Metal LaF has the headers centered.

[GerdP]

compute minimun column based on the data

[GerdP]

@Taylor: Please try this on Mac. It works fine on my Windows machine. The new method TagInfoViewer.adjustWidths(JTable table) should probably go somewhere else as it may be of use elsewhere. The code is based on a snippet I found here: ​https://tips4java.wordpress.com/2008/11/10/table-column-adjuster/

[taylor.smock]

I'll give the patch a shot.

Quick comments on the patch:

• Why are you assigning a variable that you just return in buildReferenceTable and buildCurrentTable? NVM. I missed the new fields.
• Where did this.getWidth() / 4 come from? Shouldn't it either be 3 or 6 (why not get the number of cols for the table for that matter?)
• What are the performance implications for objects with a lot of tags? (Might not be an issue)

[GerdP]

• The variables are used in adjustWidths();
• The maxwidth stuff helps when the value column contains very long strings, without it the other two colums are too small. The width is that of TagInfoViewer which contains the two tables and my thought was that no column should take more than 1/4 of that width.

The value should probably be calculated in a different way / at a different place.

• I saw no performance issues

[taylor.smock]

Replying to GerdP:

Please try this on Mac

Seems to work.

The new method [...] should probably go somewhere else

Probably TableHelper. But it looks like we already have something similar; TableHelper.computeColumnsWidth. I'll check and see if we can replace the adjustWidths call in TagInfoViewer with it.

EDIT: It looks like it works on my machine.

[GerdP]

Try also relation r62422, this has both many keys and long values. With the code in TableHelper the "Value" column is a bit too large and thus the header "Since" is truncated and some keys are not fully shown.

Not sure what is more important here and maybe Dieters suggestion to wrap columns is possible? It probably requires a lot of code for the diff view.

[taylor.smock]

Dieters suggestion to wrap columns is possible

Technically possible if we use JTextArea. The question is how hard it will be. And we'll want to sync the height between the tables. Example:

        for (int i = 0; i < Math.min(reference.getRowCount(), current.getRowCount()); i++) {
            final int height = Math.max(reference.getRowHeight(i), current.getRowHeight(i));
            reference.setRowHeight(i, height);
            current.setRowHeight(i, height);
        }

With the code in TableHelper the "Value" column is a bit too large and thus the header "Since"

Looks like it. :(

[GerdP]

Patch doesn work, it doesn't show modified tags for updated objects :(
This doesn't happen when I remove the change in HistoryBrowserDialogManager, but without that line the adjusting only happens when I click on e.g. the last row in the left pane to trigger an event. I'll try to find a better solution to trigger that.

[GerdP]

improved patch that uses existing code in TableHelper and also works with modified data

[taylor.smock]

Works for me.

[GerdP]

OK, I've mapped with this patch today and did not see any problem. I've experiemented with JTextArea in the TableCellRenderer but without success so far, so I think it should be committed as a step forward.

[GerdP]

In 19013/josm:

see #23482: optimize the space in the history view (column width) and consider adding line wrapping

• use TableHelper.adjustColumnWidth to adjust column widths each time when the displayed tag data changes
• set maximum width for the "Since" column

[Adrian]

Screenshot showing unequal split of right-hand pane

[Adrian]

Maybe this should be a separate ticket, but it seems to make sense to address this issue at the same time. The attached screenshot shows unequal columns (r19015). This makes it difficult to compare the two versions of the object. To reproduce:
Download the history of relation 7317843.
Click on version 7 in column A and then click on version 8.
Suggested solutions:
Always split the right-hand pane into two equal halves, or
Make the widths of the two halves adjustable by the user.

[GerdP]

I think I can reproduce.
The left pane is enlarged each time when the method tagInfoViewer.adjustWidths() is called. This happens when you click to compare v7 with v8 and than v8 with v9 and than again v7 with v8 and so on. Didn't recognize this before.

[GerdP]

It also happens with tested version, so it seems not directly related to the changes in r19013.

[GerdP]

This behaviour depends on the texts for the changeset, e.g. the comment or the imageary source.

[Adrian]

This behaviour has existed for a long time. I hadn't reported it until now.

[GerdP]

I've experimented with this for many hours but could not find a good solution so far.
The patch changes the layout again if the calculated layout results in unequal sizes, but that causes a flicker.

[GerdP]

hack to circumvent the growing of left or right pane

[GerdP]

The problem also disappears with this commented line:

src/org/openstreetmap/josm/gui/history/VersionInfoPanel.java

@@ -306 +306 @@
         final String text = cs != null ? cs.get(attr) : null;
         // Update text, hide prefixing label if empty
         if (label != null) {
-            label.setVisible(text != null && !Utils.isStripEmpty(text));
+            //label.setVisible(text != null && !Utils.isStripEmpty(text));
         }
         textArea.setText(text);
         // Hide container if values of both versions are empty}}}

This code was introduced with r6865 and the related ticket #9659 mentions misaligned tables before, so maybe this was committed accidentially.

[GerdP]

avoid misaligned tables

[GerdP]

@Taylor: Can you please try this on MacOS? I see no disadvantage with the patched version. The original code may show an empty line when no source was given, the patched code shows a line with Source:
Advantage is that the column widths do not change.

[Adrian]

screenshot with 23482-3.patch

[Adrian]

I tried 23482-3.patch (with r19017) on MacOS. Mostly, it works well. I found one situation where you get unequal columns, see attached screenshot. Now the left-hand column is narrower. To reproduce:
Download the history of relation 7317843.
Click on the small triangle pointing to the right, to expand the left-hand pane to full width.
Click on the small triangle pointing to the left, to return the division between the panes to where it was.
A screen colour picker, which shows an enlarged version of the area around the cursor, and reads out the coordinates of the cursor, could be useful here (to measure elements of the history window).

I also tried with relation 62422. With the history window enlarged to full screen, the "Key" column is slightly too wide but that is not really a problem. I enlarged the window by using the window control, rather than by dragging the edges of the window. I restored the window to its original size, again using the window control. The "Since" column was then too narrow; but only if I had changed the selection in column A or B while the window was at full screen. After restoring the window, if I changed the selection again, the width of the "Since" column was reset and the version numbers reappeared. There was a similar unexpected behaviour if I moved the boundary between the "Key" and "Value" columns (by dragging the dividing line). The boundary moved back to its previous position when I changed the selection. These things are probably not important.

[GerdP]

Thanks for testing. I didn't even recognize those small triangles! I'll try to find out why this happens.

Reg. the effects with r62422: Do you only see that with 23482-3.patch applied or also without? On Windows I don't seem to have a control to enlarge the popup to full screen (it is disabled), I can only drag the edges. I still can reproduce the too narrow "Since" column. Working on it...

[GerdP]

Reg. the problem with the triangles: When I repeat the two steps the sizes are correct. Next time they are again wrong. And so on.
Still no idea what's going on there.

Regarding the too narrow "Since" column: I think we have to live with that. Users probably don't resize the dialog so that the values cannot be displayed properly.

[Adrian]

Replying to GerdP:

Reg. the effects with r62422: Do you only see that with 23482-3.patch applied or also without? On Windows I don't seem to have a control to enlarge the popup to full screen (it is disabled), I can only drag the edges.

On MacOS, one of the window controls is disabled, the equivalent of minimising the window to the taskbar.

With r19015, without patch, with relation 62422:

When the history window opens, the "Since" columns are wide enough. After enlarging the window to full screen, the "Key" column is too wide, wider than with the patch. After restoring the window, its appearance is the same as when the window opened. If the selection is changed while the window is at full screen, the "Key" column becomes the same width as with the patch. When the window is restored after changing the selection, the columns are unequal, with the left-hand column being wider. The "Since" column is too narrow. Then change the selection, and the "Since" column becomes wider (similarly to with the patch), and the inequality between the columns becomes worse. Drag the division between the "Key" and "Value" columns, change the selection, and the division moves back to where it was, the same as with the patch.

I have discovered that the behaviour is different, the first time that the history window is opened after launching JOSM. Without the patch:
The "Since" columns are too narrow. Change the selection, and the "Since" column becomes wider, and the columns become unequal. Enlarge to full screen and then restore, and the inequality between the columns is reduced, and both "Since" columns are wide enough.
With the patch:
The "Since" columns are too narrow. Change the selection, and the "Since" columns become wider, wide enough to show all the version numbers. After enlarging the window to full screen, the "Key" column is too wide, the same as without the patch. Don't change the selection, restore the window, and its appearance is the same as before it was enlarged. If the selection is changed while the window is at full screen, the "Key" column becomes narrower, the same width as with subsequent history windows. Restore the window after changing the selection, and the "Since" columns are too narrow. Change the selection again, and the "Since" columns become wide enough.

[GerdP]

OK, if I got that right 23482-3.patch is an approvement and it doesn't introduce further regressions? The patch only addresses the unequal sizes of left and right, not the columns widths.

The switch between full size and restored size may show the same problem as the toggle with the triangle control. It is easy to disable the triangle control, but that's not really a solution ;)
My findings so far:
The java layouter seems to have a problem with the calculation of the component widths when there is a JTextArea: Sometimes it decides to wrap the line of a long comment, sometimes it doesn't. Depending on this the left/right sides can be unequal and I've not found any way to force equal widths without also forcing a height, and the latter always results in completely wrong layout :(

[GerdP]

I just found another problematic case: node 343494089 v5 : mapper name is extremely long and it is not wrapped.

[Adrian]

Replying to GerdP:

OK, if I got that right 23482-3.patch is an approvement and it doesn't introduce further regressions?

Yes, it is a good improvement. And as far as I can see, it does not make anything worse. I think it is worth committing, even if you may be making more changes.

[Adrian]

Replying to GerdP:

I just found another problematic case: node 343494089 v5 : mapper name is extremely long and it is not wrapped.

I noticed that the other piece of clickable text, the changeset number, can also be truncated. Two possible reasons why wrapping fails:

• The text does not contain a space character.
• It's because the text is clickable.

If it is because the text is clickable, there may be a workaround.

• Convert the text to ordinary text.
• Move the click functionality and the tooltip text to a new object, e.g. a small globe icon.

[taylor.smock]

Ticket retargeted after milestone closed

[GerdP]

In 19020/josm:

see #23482: optimize the space in the history view (column width) and consider adding line wrapping

• avoid misaligned tables caused by invisible fields (in many but not all cases)

[taylor.smock]

Ticket retargeted after milestone closed

[taylor.smock]

Ticket retargeted after milestone closed