Modify

Opened 5 years ago

Last modified 5 months ago

#17295 new enhancement

[WIP 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 years ago.
error-then-warning.webm (345.9 KB ) - added by bagage 5 years ago.
small video of some issues
17295-v2.patch (6.4 KB ) - added by GerdP 5 years ago.
17295-v3.patch (5.1 KB ) - added by GerdP 5 years ago.
adapted to r14899

Download all attachments as: .zip

Change History (15)

comment:1 by bagage, 5 years ago

Description: modified (diff)

comment:2 by GerdP, 5 years ago

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 by bagage, 5 years ago

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.

Version 0, edited 5 years ago by bagage (next)

by GerdP, 5 years ago

Attachment: 17295.patch added

comment:4 by GerdP, 5 years ago

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 by GerdP, 5 years ago

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

comment:6 by bagage, 5 years ago

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.

by bagage, 5 years ago

Attachment: error-then-warning.webm added

small video of some issues

in reply to:  6 comment:7 by GerdP, 5 years ago

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.

by GerdP, 5 years ago

Attachment: 17295-v2.patch added

comment:8 by GerdP, 5 years ago

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 ...

by GerdP, 5 years ago

Attachment: 17295-v3.patch added

adapted to r14899

comment:9 by GerdP, 5 years ago

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 by bagage, 5 years ago

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).

comment:11 by gaben, 5 months ago

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

Showing the validator list expanded is a good addition on its own, saving a lot of clicks.

But what if I want to see how many issues there are without touching anything? After this patch, I have to close the error list if it's long enough (not fitting in the view).

Small things make a UI good and easy to use, and I have no better solution either :/

Modify Ticket

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

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.