Modify

Opened 16 years ago

Closed 16 years ago

#2905 closed enhancement (fixed)

[new PATCH] Add option to disable the new "Move elements" dialog

Reported by: avarab@… Owned by: Gubaer
Priority: major Milestone:
Component: Core Version: latest
Keywords: Cc:

Description

I often accidentally move objects in JOSM but since [1712] undoing them is painful since I have to deal with a dialog as opposed to instantly pressing C-z.

This patch adds a "Don't show this warning again" button to the dialog, I also rephrased the warning from "You did move more than {0} elements." which is an odd way of putting things to "You moved more than {0} elements.".

Attachments (4)

undo-move-dialog.patch (1.8 KB ) - added by avarab@… 16 years ago.
Patch to make undo move dialog optional
undo-move-dialog-2.patch (2.1 KB ) - added by avarab@… 16 years ago.
Improved patch
ExtendedDialog-dontShowAgain.patch (14.6 KB ) - added by xeen 16 years ago.
ExtendedDialog-dontShowAgain.2.patch (14.6 KB ) - added by xeen 16 years ago.
I dream of a version control system that does not suck… is it wrong to dream of git?

Download all attachments as: .zip

Change History (16)

by avarab@…, 16 years ago

Attachment: undo-move-dialog.patch added

Patch to make undo move dialog optional

by avarab@…, 16 years ago

Attachment: undo-move-dialog-2.patch added

Improved patch

comment:1 by avarab@…, 16 years ago

Actually it's probably a good idea to revert when the user closes the dialog itself attachment:undo-move-dialog-2.patch implements that.

The user also gets a message now explaining how to re-enable the dialog in Advanced Preferences.

comment:2 by stoecker, 16 years ago

We already ha a don't show me againdialog. This one should be used instead of a new form of dialog.

comment:3 by stoecker, 16 years ago

Owner: changed from team to avarab@…
Status: newneedinfo

in reply to:  2 comment:4 by anonymous, 16 years ago

Replying to stoecker:

We already ha a don't show me againdialog. This one should be used instead of a new form of dialog.

I'm a very casual JOSM patch submitter and greping through the source I couldn't find a way to use that dialog in a way that it showed 3 buttons not just "Ok" and "Don't show me again.

In the time that I wanted to spend on it that is, and I also don't know Java:)

in reply to:  2 comment:5 by avarab@…, 16 years ago

Replying to stoecker:

We already ha a don't show me againdialog. This one should be used instead of a new form of dialog.

The dialog currently being used is better in this case than the generic DontShowAgainInfo dialog:

  • It uses ExtendedDialog but not JOptionPane.showConfirmDialog so
    • It's possible to set a dialog title
    • There are buttons that reflect their function ("Move them"/"Undo move") instead of generic OK/Cancel buttons
  • This dialog is associated with a preference option that's not boolean which the DontShowAgainInfo class enforces

comment:6 by stoecker, 16 years ago

The "Don't show me again" checkbox is (even if I dislike it) established behaviour nowadays. Thus independent from the way it is done in the code, the way it is represented to the user must be identical. That's why I still reject that patch. BTW the improved patch misses the essential line to set value to -1.

comment:7 by xeen, 16 years ago

Summary: [PATCH] Add option to disable the new "Move elements" dialog[new PATCH] Add option to disable the new "Move elements" dialog

The dialog stoecker meant is called "ConditionalOptionPaneUtil", which is hard to grep indeed. The only advantage of it compared to Java's integrated dialog is the "Don't show again" checkbox, so it doesn't allow for nice dialogs.

I instead extended the ExtendedDialog once again and refactored it a bit on the way. The checkbox implementation is compatible to ConditionalOptionPaneUtil, but the checkbox text can be modified. The refactoring had to be done because the constructors were already confusing enough and adding another option to them would be terribile to maintain. The patch deprecates all old constructors and features a minimal constructor that only requests those variables that need to be set due to technical reasons. All other features can be enabled by calling separate functions which is easier extendable and also better readable.

The patch updates the Move-Warning to use the new ExtendedDialog features.

comment:8 by xeen, 16 years ago

Owner: changed from avarab@… to stoecker
Status: needinfonew

(I'm also suggesting to deprecate "ConditionalOptionPaneUtil". There do net seem to be too many users anyway:

find . -iname "*.java" -exec grep "import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil" -l {} \;
./JOSM/src/org/openstreetmap/josm/actions/OrthogonalizeAction.java
./JOSM/src/org/openstreetmap/josm/command/DeleteCommand.java
./JOSM/src/org/openstreetmap/josm/gui/layer/RawGpsLayer.java
./JOSM/src/org/openstreetmap/josm/gui/layer/GpxLayer.java
./JOSM/src/org/openstreetmap/josm/gui/dialogs/LayerListDialog.java
./JOSM/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java

)

comment:9 by Gubaer, 16 years ago

Owner: changed from stoecker to xeen

I'd be interested to replace the current uses of ConditionalOptionPaneUtil with the new ExtendedDialog.

Unfortunatelly your patch doesn't apply anymore in r2028. Could you please submit a new version?

by xeen, 16 years ago

I dream of a version control system that does not suck… is it wrong to dream of git?

comment:10 by xeen, 16 years ago

Owner: changed from xeen to Gubaer

hm, not too much seems to have changed, please try the new patch. It's again 2030, so hopefully it works :)

comment:11 by Gubaer, 16 years ago

applied in r2031

comment:12 by stoecker, 16 years ago

Resolution: fixed
Status: newclosed

As I understand it, git can be used to control SVN and some people also use it for JOSM.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Gubaer.
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.