diff --git a/src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java b/src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java index 959f73d1d0..1c10a7c575 100644 --- a/src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java +++ b/src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java @@ -154,7 +154,7 @@ public class RelationChecker extends Test implements TaggingPresetListener { .message(tr("Route scheme is unspecified. Add {0} ({1}=public_transport; {2}=legacy)", "public_transport:version", "2", "1")) .primitives(n) .build()); - } else if (n.hasKey("type") && allroles.isEmpty()) { + } else if ((n.hasKey("type") && allroles.isEmpty()) || !n.hasKey("type")) { errors.add(TestError.builder(this, Severity.OTHER, RELATION_UNKNOWN) .message(tr("Relation type is unknown")) .primitives(n) diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java b/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java index 743e05f4e0..c5e10ad531 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java +++ b/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java @@ -3,6 +3,7 @@ package org.openstreetmap.josm.gui.dialogs.relation; import static org.openstreetmap.josm.gui.help.HelpUtil.ht; import static org.openstreetmap.josm.tools.I18n.tr; +import static org.openstreetmap.josm.tools.I18n.trn; import java.awt.BorderLayout; import java.awt.Component; @@ -26,9 +27,12 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.EnumSet; import java.util.List; import java.util.Set; +import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; import javax.swing.AbstractAction; @@ -47,6 +51,8 @@ import javax.swing.JTabbedPane; import javax.swing.JTable; import javax.swing.JToolBar; import javax.swing.KeyStroke; +import javax.swing.event.TableModelListener; +import javax.swing.table.TableModel; import org.openstreetmap.josm.actions.JosmAction; import org.openstreetmap.josm.command.ChangeMembersCommand; @@ -54,10 +60,12 @@ import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.data.UndoRedoHandler; import org.openstreetmap.josm.data.UndoRedoHandler.CommandQueueListener; import org.openstreetmap.josm.data.osm.DefaultNameFormatter; +import org.openstreetmap.josm.data.osm.IRelation; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.RelationMember; import org.openstreetmap.josm.data.osm.Tag; +import org.openstreetmap.josm.data.validation.TestError; import org.openstreetmap.josm.data.validation.tests.RelationChecker; import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil; import org.openstreetmap.josm.gui.MainApplication; @@ -105,9 +113,12 @@ import org.openstreetmap.josm.gui.tagging.presets.TaggingPreset; import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetHandler; import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetType; import org.openstreetmap.josm.gui.tagging.presets.TaggingPresets; +import org.openstreetmap.josm.gui.util.GuiHelper; import org.openstreetmap.josm.gui.util.WindowGeometry; import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.tools.CheckParameterUtil; +import org.openstreetmap.josm.tools.GBC; +import org.openstreetmap.josm.tools.ImageProvider; import org.openstreetmap.josm.tools.InputMapUtils; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Shortcut; @@ -132,6 +143,7 @@ public class GenericRelationEditor extends RelationEditor implements CommandQueu private final SelectionTableModel selectionTableModel; private final AutoCompletingTextField tfRole; + private final RelationEditorActionAccess actionAccess; /** * the menu item in the windows menu. Required to properly hide on dialog close. @@ -262,13 +274,24 @@ public class GenericRelationEditor extends RelationEditor implements CommandQueu selectedTabPane = sourceTabbedPane.getSelectedComponent(); }); - IRelationEditorActionAccess actionAccess = new RelationEditorActionAccess(); + actionAccess = new RelationEditorActionAccess(); refreshAction = new RefreshAction(actionAccess); applyAction = new ApplyAction(actionAccess); selectAction = new SelectAction(actionAccess); duplicateAction = new DuplicateRelationAction(actionAccess); deleteAction = new DeleteCurrentRelationAction(actionAccess); + + this.memberTableModel.addTableModelListener(applyAction); + this.tagEditorPanel.getModel().addTableModelListener(l -> { + // Short circuit -- the tag editor will fire an update event on selection change, even if nothing actually + // changed. + if (l != null && l.getSource() instanceof TagEditorModel && !((TagEditorModel) l.getSource()).isDirty()) { + return; + } + applyAction.tableChanged(l); + }); + addPropertyChangeListener(deleteAction); okAction = new OKAction(actionAccess); @@ -276,7 +299,7 @@ public class GenericRelationEditor extends RelationEditor implements CommandQueu getContentPane().add(buildToolBar(refreshAction, applyAction, selectAction, duplicateAction, deleteAction), BorderLayout.NORTH); getContentPane().add(tabbedPane, BorderLayout.CENTER); - getContentPane().add(buildOkCancelButtonPanel(okAction, cancelAction), BorderLayout.SOUTH); + getContentPane().add(buildOkCancelButtonPanel(okAction, deleteAction, cancelAction), BorderLayout.SOUTH); setSize(findMaxDialogSize()); @@ -407,12 +430,89 @@ public class GenericRelationEditor extends RelationEditor implements CommandQueu * * @return the panel with the OK and the Cancel button */ - protected static JPanel buildOkCancelButtonPanel(OKAction okAction, CancelAction cancelAction) { - JPanel pnl = new JPanel(new FlowLayout(FlowLayout.CENTER)); - pnl.add(new JButton(okAction)); + protected JPanel buildOkCancelButtonPanel(OKAction okAction, DeleteCurrentRelationAction deleteAction, + CancelAction cancelAction) { + JPanel mainPanel = new JPanel(new GridBagLayout()); + final JLabel errorLabel = new JLabel(" "); + final JPanel pnl = new JPanel(new FlowLayout(FlowLayout.CENTER)); + mainPanel.add(errorLabel, GBC.eol().anchor(GridBagConstraints.CENTER)); + mainPanel.add(pnl, GBC.eol().anchor(GridBagConstraints.CENTER)); + final JButton okButton = new JButton(okAction); + final JButton deleteButton = new JButton(deleteAction); + okButton.setPreferredSize(deleteButton.getPreferredSize()); + pnl.add(okButton); + pnl.add(deleteButton); pnl.add(new JButton(cancelAction)); pnl.add(new JButton(new ContextSensitiveHelpAction(ht("/Dialog/RelationEditor")))); - return pnl; + // Keep users from saving invalid relations -- a relation MUST have at least a tag with the key "type" + // AND must contain at least one other OSM object. + final AtomicReference> testErrorsUpdate = new AtomicReference<>(); + final TableModelListener listener = l -> { + final IRelation newRelation = this.actionAccess.getChangedRelation(); + updateOkPanel(newRelation, okButton, deleteButton); + final Future oldFuture = testErrorsUpdate.getAndSet(MainApplication.worker.submit(() -> { + GuiHelper.runInEDTAndWait(() -> errorLabel.setText(tr("Validation in progress"))); + final Collection testErrors = newRelation.validate(); + GuiHelper.runInEDTAndWait(() -> updateErrorMessage(testErrors, errorLabel)); + })); + if (oldFuture != null) { + oldFuture.cancel(true); + } + }; + listener.tableChanged(null); + this.memberTableModel.addTableModelListener(listener); + this.tagEditorPanel.getModel().addTableModelListener(l -> { + // Short circuit -- the tag editor will fire an update event on selection change, even if nothing actually + // changed. + if (l != null && l.getSource() instanceof TagEditorModel && !((TagEditorModel) l.getSource()).isDirty()) { + return; + } + listener.tableChanged(l); + }); + return mainPanel; + } + + /** + * Update the OK panel area + * @param newRelation What the new relation would "look" like if it were to be saved now + * @param okButton The OK button + * @param deleteButton The delete button + */ + private void updateOkPanel(IRelation newRelation, JButton okButton, JButton deleteButton) { + okButton.setVisible(newRelation.isUseful() || this.getRelationSnapshot() == null); + deleteButton.setVisible(!newRelation.isUseful() && this.getRelationSnapshot() != null); + if (this.getRelationSnapshot() == null && !newRelation.isUseful()) { + okButton.setText(tr("Delete")); + } else { + okButton.setText(tr("OK")); + } + } + + /** + * Update the error message + * @param newRelation What the new relation would "look" like if it were to be saved now + * @param errorLabel The label to update + */ + private void updateErrorMessage(Collection errors, JLabel errorLabel) { + if (errors.isEmpty()) { + // Setting " " helps keep the label in place for layout calculations + errorLabel.setText(" "); + errorLabel.setIcon(null); + } else { + final TestError error = errors.stream() + .min(Comparator.comparingInt(testError -> testError.getSeverity().getLevel())) + .orElse(errors.iterator().next()); + final StringBuilder sb = new StringBuilder(); + if (error.getDescription() != null) { + sb.append(error.getDescription()).append(": "); + } + sb.append(error.getMessage()); + if (errors.size() > 1) { + sb.append(trn(" with {0} more problem", " with {0} more problems", errors.size() - 1, errors.size() - 1)); + } + errorLabel.setIcon(ImageProvider.get("data", error.getSeverity().getIcon())); + errorLabel.setText(sb.toString()); + } } /**