Modify

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#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 GerdP)

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)

19257.patch (5.4 KB ) - added by GerdP 4 years ago.
19257.2.patch (6.8 KB ) - added by GerdP 4 years ago.
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.

Download all attachments as: .zip

Change History (13)

comment:1 by GerdP, 4 years ago

Description: modified (diff)

comment:2 by GerdP, 4 years ago

Owner: changed from team to GerdP
Status: newassigned

by GerdP, 4 years ago

Attachment: 19257.patch added

comment:3 by GerdP, 4 years ago

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

comment:4 by simon04, 4 years ago

I also couldn't find a plugin using ChangesetContentPanel. Let's remove it.

comment:5 by GerdP, 4 years ago

In 16461/josm:

see #19257: remove duplicated code in different implementations of ShowHistoryAction

comment:6 by GerdP, 4 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 GerdP, 4 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 simon04, 4 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 GerdP, 4 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:9 by GerdP, 4 years ago

In 16495/josm:

see #19257: reduce number of ShowHistoryAction implementations
Add 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:10 by GerdP, 4 years ago

Resolution: fixed
Status: assignedclosed

I think we need the remaining variants.

comment:11 by GerdP, 4 years ago

Milestone: 20.05

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain GerdP.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


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