Modify

Opened 2 months ago

Last modified 5 weeks ago

#17295 new enhancement

[Patch] All categories are folded by default

Reported by: bagage Owned by: team
Priority: normal Milestone:
Component: Core validator Version:
Keywords: template_report Cc:

Description (last modified by bagage)

What steps will reproduce the problem?

  1. Open josm, download some OSM data
  2. Launch validator

What is the expected result?

Warnings/errors are displayed, stored by categories. You can review them quickly.

What happens instead?

Warnings/errors are generated, but these 2 panels (error/warning) are folded. So you cannot see all categories easily, you have to click on error/warning. Then next validator action will leave it open.

I don't see the value of folding error/warning categories at start, because by itself it's not very useful I believe.
But even better, I'd like if JOSM could save my preferences on which categories (Crossing highways, …) I opened, so that next time I open JOSM, it will open these directly without me having to click.

Please provide any additional information below. Attach a screenshot if possible.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2018-12-31 15:09:58 +0100 (Mon, 31 Dec 2018)
Build-Date:2018-12-31 14:24:10
Revision:14620
Relative:URL: ^/trunk

Identification: JOSM/1.5 (14620 fr) Linux Debian GNU/Linux buster/sid
Memory Usage: 1127 MB / 4436 MB (804 MB allocated, but free)
Java version: 1.8.0_191-8u191-b12-2-b12, Oracle Corporation, OpenJDK 64-Bit Server VM
Screen: :0.0 1920x1080
Maximum Screen Size: 1920x1080
Java package: openjdk-8-jre:amd64-8u191-b12-2
Java ATK Wrapper package: libatk-wrapper-java:all-0.33.3-21
Dataset consistency test: No problems found

Plugins:
+ AddrInterpolation (34867)
+ HouseNumberTaggingTool (34867)
+ OpeningHoursEditor (34867)
+ PicLayer (34867)
+ apache-commons (34506)
+ buildings_tools (34867)
+ conflation (0.6.2)
+ ejml (34389)
+ geojson (116)
+ geotools (34513)
+ jts (34524)
+ poly (34867)
+ scripting (30794)
+ terracer (34867)
+ todo (30306)
+ utilsplugin2 (34867)

Last errors/warnings:
- W: No configuration settings found.  Using hardcoded default values for all pools.

Attachments (4)

17295.patch (2.2 KB) - added by GerdP 5 weeks ago.
error-then-warning.webm (345.9 KB) - added by bagage 5 weeks ago.
small video of some issues
17295-v2.patch (6.4 KB) - added by GerdP 5 weeks ago.
17295-v3.patch (5.1 KB) - added by GerdP 5 weeks ago.
adapted to r14899

Download all attachments as: .zip

Change History (14)

comment:1 Changed 2 months ago by bagage

Description: modified (diff)

comment:2 Changed 2 months ago by GerdP

I agree that it doesn't make much sense to show the collapsed tree. I also like your 2nd idea to have a "preferred" errors list.
See also #17268 where we are working on a better handling of the ignored errors, maybe your approach would be an alternative.
So, instead of adding errors to the ignored list (so that they somehow disappear) we might be more effective when we build the tree in a way so that interesting (or favorite?) items are moved to the top.

comment:3 Changed 2 months ago by bagage

Indeed #17268 will definitely help on that issue, it's great to be able of restoring ignored errors!

I believe that for this issue, the best behavior IMHO would be to:

1) put preferred categories first in error/warning lists.
2) put ignored categories in a third list such as "ignored", folded. You can thus still see the number of ignored warnings for these, but you can just ignore that and it won't take much space in the UI. The "ignore" button should be disabled when an item is coming from this list obviously. Or even better, it could be changed to "Reenable". But maybe that would break the whole logic behind #17268?

Last edited 2 months ago by bagage (previous) (diff)

Changed 5 weeks ago by GerdP

Attachment: 17295.patch added

comment:4 Changed 5 weeks ago by GerdP

With the attached patch the tree is always expanded. In cases where the unpatched code would should the tree completely collapsed the new code expands the first severity level. If that results in a tree with less than 10 rows it tries to expand also the next lower severity level.
I don't know yet how to implement the "preferred errors", this would require much more logic.
Please suggest improvements, else I'll commit this patch tomorrow.

comment:5 Changed 5 weeks ago by GerdP

Summary: All categories are folded by default[Patch] All categories are folded by default

comment:6 Changed 5 weeks ago by bagage

Thanks for the patch GerdP! It's indeed nice to see categories directly now.

If that results in a tree with less than 10 rows it tries to expand also the next lower severity level.

I'm not sure to understand this fully: is it supposed to open first category (for instance "Crossing roads") directly if there are less than 10 different categories? If so I believe it does not work for me.

Similarly I have a small issue when I run validation a first time which shows only warnings. Then a new validation that also triggers errors: in that case, errors are not displayed.

See this small video for example of both issues.

Changed 5 weeks ago by bagage

Attachment: error-then-warning.webm added

small video of some issues

comment:7 in reply to:  6 Changed 5 weeks ago by GerdP

Replying to bagage:

Thanks for the patch GerdP! It's indeed nice to see categories directly now.

If that results in a tree with less than 10 rows it tries to expand also the next lower severity level.

I'm not sure to understand this fully: is it supposed to open first category (for instance "Crossing roads") directly if there are less than 10 different categories? If so I believe it does not work for me.

No, it is intended to open severity (errors/warnings/other)

Similarly I have a small issue when I run validation a first time which shows only warnings. Then a new validation that also triggers errors: in that case, errors are not displayed.

See this small video for example of both issues.

Ah, yes, if you mean that errors are not expanded that would be an unwanted side effect of the patch. It remembers the expanded row and with the new tree it expands the entry which was automatically expanded with the first validation. Maybe I can implement a flag so that only manual exapandations are remembered.
On the other hand I think a similar problem also exists in the unpatched version. If you expand warnings and the new validation produces also errors only the warnings are expanded.

Changed 5 weeks ago by GerdP

Attachment: 17295-v2.patch added

comment:8 Changed 5 weeks ago by GerdP

With v2 the program tries to separate manually expanded tree rows from automatically expanded rows. This requires quite a lot of "not so nice" code, so I am not very happy with that, and probably I did not yet catch all problems.
I am also not yet happy with the "re-selection" , it should try to re-select the same issue, not the same row. Working on this ...

Changed 5 weeks ago by GerdP

Attachment: 17295-v3.patch added

adapted to r14899

comment:9 Changed 5 weeks ago by GerdP

I thought it would be a simple change but it turned out to have a lot of side effects. I don't think that this patch should be committed, it still uses some tricks and it is rather difficult to guess what a user wants to get when the selection is changed and validation is executed again. In what cases would you want the previously expanded rows expanded again?
It would be easier if we would either always try to expand what was expanded before or always only expand the severities.
At least this would reduce the needed tricks.
Maybe I'll try again to implement the idea of "preferred errors" later.

comment:10 Changed 5 weeks ago by bagage

Well I wasn't aware of such edge cases (I encountered a lot of issues like that on the tags values table autoresizing issue…).

In that case and as far as I can tell, simply having the whole tree open by default (eg errors and warnings expanded) when you run the validator would be fine. Only the first time ideally otherwise everytime? (but that would reopen folded categories maybe).

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set.
to The owner will be changed from team to the specified user.
The owner will change to bagage
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket
The owner will be changed from team to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.