Index: trunk/src/org/openstreetmap/josm/gui/io/BasicUploadSettingsPanel.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/io/BasicUploadSettingsPanel.java (revision 16671) +++ trunk/src/org/openstreetmap/josm/gui/io/BasicUploadSettingsPanel.java (revision 16672) @@ -4,5 +4,4 @@ import static org.openstreetmap.josm.tools.I18n.tr; -import java.awt.BorderLayout; import java.awt.GridBagLayout; import java.awt.event.ActionEvent; @@ -20,4 +19,5 @@ import javax.swing.BorderFactory; +import javax.swing.BoxLayout; import javax.swing.JCheckBox; import javax.swing.JEditorPane; @@ -76,8 +76,6 @@ protected JPanel buildUploadCommentPanel() { JPanel pnl = new JPanel(new GridBagLayout()); - pnl.setBorder(BorderFactory.createTitledBorder(tr("Tags of changeset {0}", ""))); - - JEditorPane commentLabel = new JMultilineLabel("" + tr("Provide a brief comment for the changes you are uploading:")); - pnl.add(commentLabel, GBC.eol().insets(0, 5, 10, 3).fill(GBC.HORIZONTAL)); + pnl.setBorder(BorderFactory.createTitledBorder(tr("Provide a brief comment for the changes you are uploading:"))); + hcbUploadComment.setToolTipText(tr("Enter an upload comment")); hcbUploadComment.setMaxTextLength(Changeset.MAX_CHANGESET_TAG_LENGTH); @@ -87,7 +85,14 @@ hcbUploadComment.getEditorComponent().addFocusListener(commentModelListener); pnl.add(hcbUploadComment, GBC.eol().fill(GBC.HORIZONTAL)); - - JEditorPane sourceLabel = new JMultilineLabel("" + tr("Specify the data source for the changes") + ":"); - pnl.add(sourceLabel, GBC.eol().insets(0, 8, 10, 0).fill(GBC.HORIZONTAL)); + JLabel hcbUploadCommentFeedback = new JLabel(); + pnl.add(hcbUploadCommentFeedback, GBC.eol().insets(0, 3, 0, 0).fill(GBC.HORIZONTAL)); + new UploadTextComponentValidator.UploadCommentValidator(hcbUploadComment.getEditorComponent(), hcbUploadCommentFeedback); + return pnl; + } + + protected JPanel buildUploadSourcePanel() { + JPanel pnl = new JPanel(new GridBagLayout()); + pnl.setBorder(BorderFactory.createTitledBorder(tr("Specify the data source for the changes"))); + JEditorPane obtainSourceOnce = new JMultilineLabel( "(" + tr("just once") + ")"); @@ -119,4 +124,7 @@ hcbUploadSource.getEditorComponent().addFocusListener(sourceModelListener); pnl.add(hcbUploadSource, GBC.eol().fill(GBC.HORIZONTAL)); + JLabel hcbUploadSourceFeedback = new JLabel(); + pnl.add(hcbUploadSourceFeedback, GBC.eol().insets(0, 3, 0, 0).fill(GBC.HORIZONTAL)); + new UploadTextComponentValidator.UploadSourceValidator(hcbUploadSource.getEditorComponent(), hcbUploadSourceFeedback); if (obtainSourceAutomatically.isSelected()) { automaticallyAddSource(); @@ -181,10 +189,13 @@ protected void build() { - setLayout(new BorderLayout()); + setLayout(new BoxLayout(this, BoxLayout.Y_AXIS)); setBorder(BorderFactory.createEmptyBorder(3, 3, 3, 3)); - add(buildUploadCommentPanel(), BorderLayout.NORTH); - add(pnlUploadParameterSummary, BorderLayout.CENTER); + add(buildUploadCommentPanel()); + add(buildUploadSourcePanel()); + add(pnlUploadParameterSummary); if (Config.getPref().getBoolean("upload.show.review.request", true)) { - add(cbRequestReview, BorderLayout.SOUTH); + JPanel pnl = new JPanel(new GridBagLayout()); + pnl.add(cbRequestReview, GBC.eol().fill(GBC.HORIZONTAL)); + add(pnl); cbRequestReview.addItemListener(e -> changesetReviewModel.setReviewRequested(e.getStateChange() == ItemEvent.SELECTED)); } Index: trunk/src/org/openstreetmap/josm/gui/io/UploadDialog.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/io/UploadDialog.java (revision 16671) +++ trunk/src/org/openstreetmap/josm/gui/io/UploadDialog.java (revision 16672) @@ -17,5 +17,4 @@ import java.lang.Character.UnicodeBlock; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -30,5 +29,4 @@ import javax.swing.AbstractAction; import javax.swing.BorderFactory; -import javax.swing.Icon; import javax.swing.JButton; import javax.swing.JOptionPane; @@ -41,5 +39,4 @@ import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.OsmPrimitive; -import org.openstreetmap.josm.gui.ExtendedDialog; import org.openstreetmap.josm.gui.HelpAwareOptionPane; import org.openstreetmap.josm.gui.MainApplication; @@ -57,7 +54,5 @@ import org.openstreetmap.josm.spi.preferences.Setting; import org.openstreetmap.josm.tools.GBC; -import org.openstreetmap.josm.tools.ImageOverlay; import org.openstreetmap.josm.tools.ImageProvider; -import org.openstreetmap.josm.tools.ImageProvider.ImageSizes; import org.openstreetmap.josm.tools.InputMapUtils; import org.openstreetmap.josm.tools.Utils; @@ -450,115 +445,4 @@ } - /** - * Displays a warning message indicating that the upload comment is empty/short. - * @return true if the user wants to revisit, false if they want to continue - */ - protected boolean warnUploadComment() { - return warnUploadTag( - tr("Please revise upload comment"), - tr("Your upload comment is empty, or very short.

" + - "This is technically allowed, but please consider that many users who are
" + - "watching changes in their area depend on meaningful changeset comments
" + - "to understand what is going on!

" + - "If you spend a minute now to explain your change, you will make life
" + - "easier for many other mappers."), - "upload_comment_is_empty_or_very_short" - ); - } - - /** - * Displays a warning message indicating that no changeset source is given. - * @return true if the user wants to revisit, false if they want to continue - */ - protected boolean warnUploadSource() { - return warnUploadTag( - tr("Please specify a changeset source"), - tr("You did not specify a source for your changes.
" + - "It is technically allowed, but this information helps
" + - "other users to understand the origins of the data.

" + - "If you spend a minute now to explain your change, you will make life
" + - "easier for many other mappers."), - "upload_source_is_empty" - ); - } - - /** - * Displays a warning message indicating that the upload comment is rejected. - * @param details details explaining why - * @return {@code true} - */ - protected boolean warnRejectedUploadComment(String details) { - return warnRejectedUploadTag( - tr("Please revise upload comment"), - tr("Your upload comment is rejected.") + "
" + details - ); - } - - /** - * Displays a warning message indicating that the changeset source is rejected. - * @param details details explaining why - * @return {@code true} - */ - protected boolean warnRejectedUploadSource(String details) { - return warnRejectedUploadTag( - tr("Please revise changeset source"), - tr("Your changeset source is rejected.") + "
" + details - ); - } - - /** - * Warn about an upload tag with the possibility of resuming the upload. - * @param title dialog title - * @param message dialog message - * @param togglePref preference entry to offer the user a "Do not show again" checkbox for the dialog - * @return {@code true} if the user wants to revise the upload tag - */ - protected boolean warnUploadTag(final String title, final String message, final String togglePref) { - return warnUploadTag(title, message, togglePref, true); - } - - /** - * Warn about an upload tag without the possibility of resuming the upload. - * @param title dialog title - * @param message dialog message - * @return {@code true} - */ - protected boolean warnRejectedUploadTag(final String title, final String message) { - return warnUploadTag(title, message, null, false); - } - - private boolean warnUploadTag(final String title, final String message, final String togglePref, boolean allowContinue) { - List buttonTexts = new ArrayList<>(Arrays.asList(tr("Revise"), tr("Cancel"))); - List buttonIcons = new ArrayList<>(Arrays.asList( - new ImageProvider("ok").setMaxSize(ImageSizes.LARGEICON).get(), - new ImageProvider("cancel").setMaxSize(ImageSizes.LARGEICON).get())); - List tooltips = new ArrayList<>(Arrays.asList( - tr("Return to the previous dialog to enter a more descriptive comment"), - tr("Cancel and return to the previous dialog"))); - if (allowContinue) { - buttonTexts.add(tr("Continue as is")); - buttonIcons.add(new ImageProvider("upload").setMaxSize(ImageSizes.LARGEICON).addOverlay( - new ImageOverlay(new ImageProvider("warning-small"), 0.5, 0.5, 1.0, 1.0)).get()); - tooltips.add(tr("Ignore this hint and upload anyway")); - } - - ExtendedDialog dlg = new ExtendedDialog((Component) dialog, title, buttonTexts.toArray(new String[] {})) { - @Override - public void setupDialog() { - super.setupDialog(); - InputMapUtils.addCtrlEnterAction(getRootPane(), buttons.get(buttons.size() - 1).getAction()); - } - }; - dlg.setContent("" + message + ""); - dlg.setButtonIcons(buttonIcons.toArray(new Icon[] {})); - dlg.setToolTipTexts(tooltips.toArray(new String[] {})); - dlg.setIcon(JOptionPane.WARNING_MESSAGE); - if (allowContinue) { - dlg.toggleEnable(togglePref); - } - dlg.setCancelButton(1, 2); - return dlg.showDialog().getValue() != 3; - } - protected void warnIllegalChunkSize() { HelpAwareOptionPane.showOptionDialog( @@ -614,24 +498,4 @@ // force update of model in case dialog is closed before focus lost event, see #17452 dialog.forceUpdateActiveField(); - - final List def = Collections.emptyList(); - final String uploadComment = dialog.getUploadComment(); - final String uploadCommentRejection = validateUploadTag( - uploadComment, "upload.comment", def, def, def); - if ((isUploadCommentTooShort(uploadComment) && warnUploadComment()) || - (uploadCommentRejection != null && warnRejectedUploadComment(uploadCommentRejection))) { - // abort for missing or rejected comment - dialog.handleMissingComment(); - return; - } - final String uploadSource = dialog.getUploadSource(); - final String uploadSourceRejection = validateUploadTag( - uploadSource, "upload.source", def, def, def); - if ((Utils.isStripEmpty(uploadSource) && warnUploadSource()) || - (uploadSourceRejection != null && warnRejectedUploadSource(uploadSourceRejection))) { - // abort for missing or rejected changeset source - dialog.handleMissingSource(); - return; - } /* test for empty tags in the changeset metadata and proceed only after user's confirmation. Index: trunk/src/org/openstreetmap/josm/gui/io/UploadTextComponentValidator.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/io/UploadTextComponentValidator.java (revision 16672) +++ trunk/src/org/openstreetmap/josm/gui/io/UploadTextComponentValidator.java (revision 16672) @@ -0,0 +1,113 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.gui.io; + +import static org.openstreetmap.josm.tools.I18n.tr; + +import java.util.Collections; + +import javax.swing.JLabel; +import javax.swing.text.JTextComponent; + +import org.openstreetmap.josm.gui.widgets.AbstractTextComponentValidator; +import org.openstreetmap.josm.tools.Utils; + +/** + * Input validators for {@link UploadDialog} + */ +abstract class UploadTextComponentValidator extends AbstractTextComponentValidator { + private final JLabel feedback; + + UploadTextComponentValidator(JTextComponent tc, JLabel feedback) { + super(tc); + this.feedback = feedback; + this.feedback.setOpaque(true); + validate(); + } + + @Override + protected void feedbackValid(String msg) { + msg = msg != null ? "\u2714 " + msg : null; + super.feedbackValid(msg); + feedback.setText(msg); + feedback.setForeground(VALID_COLOR); + feedback.setBackground(null); + feedback.setBorder(null); + } + + @Override + protected void feedbackWarning(String msg) { + msg = msg != null ? "" + msg : null; + super.feedbackWarning(msg); + feedback.setText(msg); + feedback.setForeground(null); + feedback.setBackground(WARNING_BACKGROUND); + feedback.setBorder(WARNING_BORDER); + } + + @Override + public boolean isValid() { + throw new UnsupportedOperationException(); + } + + /** + * Validator for the changeset {@code comment} tag + */ + static class UploadCommentValidator extends UploadTextComponentValidator { + + UploadCommentValidator(JTextComponent tc, JLabel feedback) { + super(tc, feedback); + } + + @Override + public void validate() { + String uploadComment = getComponent().getText(); + if (UploadDialog.UploadAction.isUploadCommentTooShort(uploadComment)) { + feedbackWarning(tr("Your upload comment is empty, or very short.

" + + "This is technically allowed, but please consider that many users who are
" + + "watching changes in their area depend on meaningful changeset comments
" + + "to understand what is going on!

" + + "If you spend a minute now to explain your change, you will make life
" + + "easier for many other mappers.").replace("
", " ")); + } else { + String rejection = UploadDialog.UploadAction.validateUploadTag(uploadComment, "upload.comment", + Collections.emptyList(), Collections.emptyList(), Collections.emptyList()); + if (rejection != null) { + feedbackWarning(tr("Your upload comment is rejected.") + "
" + rejection); + } else { + feedbackValid(tr("Thank you for providing a changeset comment! " + + "This gives other mappers a better understanding of your intent.")); + } + } + } + } + + /** + * Validator for the changeset {@code source} tag + */ + static class UploadSourceValidator extends UploadTextComponentValidator { + + UploadSourceValidator(JTextComponent tc, JLabel feedback) { + super(tc, feedback); + } + + @Override + public void validate() { + String uploadSource = getComponent().getText(); + if (Utils.isStripEmpty(uploadSource)) { + feedbackWarning(tr("You did not specify a source for your changes.
" + + "It is technically allowed, but this information helps
" + + "other users to understand the origins of the data.

" + + "If you spend a minute now to explain your change, you will make life
" + + "easier for many other mappers.").replace("
", " ")); + } else { + final String rejection = UploadDialog.UploadAction.validateUploadTag( + uploadSource, "upload.source", Collections.emptyList(), Collections.emptyList(), Collections.emptyList()); + if (rejection != null) { + feedbackWarning(tr("Your changeset source is rejected.") + "
" + rejection); + } else { + feedbackValid(tr("Thank you for providing the data source!")); + } + } + } + } +} Index: trunk/src/org/openstreetmap/josm/gui/widgets/AbstractTextComponentValidator.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/widgets/AbstractTextComponentValidator.java (revision 16671) +++ trunk/src/org/openstreetmap/josm/gui/widgets/AbstractTextComponentValidator.java (revision 16672) @@ -33,22 +33,41 @@ */ public abstract class AbstractTextComponentValidator implements ActionListener, FocusListener, DocumentListener, PropertyChangeListener { - private static final Border ERROR_BORDER = BorderFactory.createLineBorder(Color.RED, 1); - private static final Color ERROR_BACKGROUND = new Color(255, 224, 224); - - private JTextComponent tc; - /** remembers whether the content of the text component is currently valid or not; null means, - * we don't know yet - */ - private Boolean valid; + protected static final Color ERROR_COLOR = Color.RED; + protected static final Border ERROR_BORDER = BorderFactory.createLineBorder(ERROR_COLOR, 1); + protected static final Color ERROR_BACKGROUND = new Color(0xFFCCCC); + protected static final Color WARNING_COLOR = new Color(0xFFA500); + protected static final Border WARNING_BORDER = BorderFactory.createLineBorder(WARNING_COLOR, 1); + protected static final Color WARNING_BACKGROUND = new Color(0xFFEDCC); + protected static final Color VALID_COLOR = new Color(0x008000); + protected static final Border VALID_BORDER = BorderFactory.createLineBorder(VALID_COLOR, 1); + + private final JTextComponent tc; + // remembers whether the content of the text component is currently valid or not; null means, we don't know yet + private Status status; // remember the message private String msg; + enum Status { + INVALID, WARNING, VALID + } + protected void feedbackInvalid(String msg) { - if (valid == null || valid || !Objects.equals(msg, this.msg)) { + if (hasChanged(msg, Status.INVALID)) { // only provide feedback if the validity has changed. This avoids unnecessary UI updates. tc.setBorder(ERROR_BORDER); tc.setBackground(ERROR_BACKGROUND); tc.setToolTipText(msg); - valid = Boolean.FALSE; + this.status = Status.INVALID; + this.msg = msg; + } + } + + protected void feedbackWarning(String msg) { + if (hasChanged(msg, Status.WARNING)) { + // only provide feedback if the validity has changed. This avoids unnecessary UI updates. + tc.setBorder(WARNING_BORDER); + tc.setBackground(WARNING_BACKGROUND); + tc.setToolTipText(msg); + this.status = Status.WARNING; this.msg = msg; } @@ -60,12 +79,16 @@ protected void feedbackValid(String msg) { - if (valid == null || !valid || !Objects.equals(msg, this.msg)) { + if (hasChanged(msg, Status.VALID)) { // only provide feedback if the validity has changed. This avoids unnecessary UI updates. - tc.setBorder(UIManager.getBorder("TextField.border")); + tc.setBorder(VALID_BORDER); tc.setBackground(UIManager.getColor("TextField.background")); tc.setToolTipText(msg == null ? "" : msg); - valid = Boolean.TRUE; + this.status = Status.VALID; this.msg = msg; } + } + + private boolean hasChanged(String msg, Status status) { + return !(Objects.equals(status, this.status) && Objects.equals(msg, this.msg)); }