#19257 closed enhancement (fixed)
reduce number of ShowHistoryAction implementations
| Reported by: | GerdP | Owned by: | GerdP |
|---|---|---|---|
| Priority: | normal | Milestone: | 20.05 |
| Component: | Core | Version: | |
| Keywords: | history | Cc: |
Description (last modified by )
follow up of #19254
We have multiple implementations of a class ShowHistoryAction
org.openstreetmap.josm.gui.dialogs.changeset.ChangesetContentPanel.ShowHistoryAction
org.openstreetmap.josm.gui.dialogs.SelectionListDialog.ShowHistoryAction
org.openstreetmap.josm.gui.history.ShowHistoryAction
They look very similar. We should have one implementation which can handle a collection of ids.
There is also org.openstreetmap.josm.actions.HistoryInfoAction which reacts on Ctrl+H.
At least they should all use HistoryBrowserDialogManager to do the real work of downloading data.
Attachments (2)
Change History (13)
comment:1 by , 6 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 6 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
by , 6 years ago
| Attachment: | 19257.patch added |
|---|
comment:3 by , 6 years ago
comment:4 by , 6 years ago
I also couldn't find a plugin using ChangesetContentPanel. Let's remove it.
comment:6 by , 5 years ago
We have an AbstractSelectAction which just contains
public AbstractSelectAction() { putValue(NAME, tr("Select")); putValue(SHORT_DESCRIPTION, tr("Selects those elements on the map which are chosen on the list above.")); new ImageProvider("dialogs", "select").getResource().attachImageIcon(this, true); }
I thought it would be good to do the same for HistoryInfoAction but I am unsure if the text for NAME should be "Show History" or just "History".
by , 5 years ago
| Attachment: | 19257.2.patch added |
|---|
AbstractShowHistoryAction with common texts "History" and "Download and show the history of the selected objects." to reduce number of I18N strings. To be used with popup menus and side buttons where selected objects are known.
comment:7 by , 5 years ago
I'm fine with just "History". For SHORT_DESCRIPTION, can we reuse a variant that is already in use (so it doesn't need to get translated again), such as "Display the history of the selected objects."?
comment:8 by , 5 years ago
I thought I did reuse an existing text, but I added a dot :(
Download and show the history of the selected objects is already used and I think it is good thing to add that data is downloaded.
comment:10 by , 5 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
I think we need the remaining variants.
comment:11 by , 5 years ago
| Milestone: | → 20.05 |
|---|



Patch removes some obsolete code. I found no plugin which uses the removed methods. Should I deprecate them first?