Opened 4 years ago
Closed 4 years ago
#2905 closed enhancement (fixed)
[new PATCH] Add option to disable the new "Move elements" dialog
| Reported by: | avar | Owned by: | Gubaer |
|---|---|---|---|
| Priority: | major | 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)
Change History (16)
Changed 4 years ago by avar
comment:1 Changed 4 years ago by avar
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 follow-ups: ↓ 4 ↓ 5 Changed 4 years ago by stoecker
We already ha a don't show me againdialog. This one should be used instead of a new form of dialog.
comment:3 Changed 4 years ago by stoecker
- Owner changed from team to avar
- Status changed from new to needinfo
comment:4 in reply to: ↑ 2 Changed 4 years ago by anonymous
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:)
comment:5 in reply to: ↑ 2 Changed 4 years ago by avar
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 Changed 4 years ago by stoecker
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.
Changed 4 years ago by xeen
comment:7 Changed 4 years ago by xeen
- Summary changed from [PATCH] Add option to disable the new "Move elements" dialog to [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 Changed 4 years ago by xeen
- Owner changed from avar to stoecker
- Status changed from needinfo to new
(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 Changed 4 years ago by Gubaer
- 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?
Changed 4 years ago by xeen
I dream of a version control system that does not suck… is it wrong to dream of git?
comment:10 Changed 4 years ago by xeen
- 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 Changed 4 years ago by Gubaer
applied in r2031
comment:12 Changed 4 years ago by stoecker
- Resolution set to fixed
- Status changed from new to closed
As I understand it, git can be used to control SVN and some people also use it for JOSM.



Patch to make undo move dialog optional