Modify

Opened 3 weeks ago

Last modified 12 days ago

#19257 assigned enhancement

reduce number of ShowHistoryAction implementations

Reported by: GerdP Owned by: GerdP
Priority: normal Milestone:
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 3 weeks ago.
19257.2.patch (6.8 KB) - added by GerdP 12 days 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 (11)

comment:1 Changed 3 weeks ago by GerdP

Description: modified (diff)

comment:2 Changed 3 weeks ago by GerdP

Owner: changed from team to GerdP
Status: newassigned

Changed 3 weeks ago by GerdP

Attachment: 19257.patch added

comment:3 Changed 3 weeks ago by GerdP

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

comment:4 Changed 3 weeks ago by simon04

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

comment:5 Changed 2 weeks ago by GerdP

In 16461/josm:

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

comment:6 Changed 12 days ago by GerdP

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

Changed 12 days ago by GerdP

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 Changed 12 days ago by simon04

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 Changed 12 days ago by GerdP

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 Changed 12 days ago by GerdP

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.

Modify Ticket

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

Add Comment


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

 
Note: See TracTickets for help on using tickets.