Changeset 16672 in josm


Ignore:
Timestamp:
2020-06-17T21:47:17+02:00 (3 weeks ago)
Author:
simon04
Message:

see #19381 - Upload dialog: make warnings less intrusive

Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/gui/io/BasicUploadSettingsPanel.java

    r16237 r16672  
    44import static org.openstreetmap.josm.tools.I18n.tr;
    55
    6 import java.awt.BorderLayout;
    76import java.awt.GridBagLayout;
    87import java.awt.event.ActionEvent;
     
    2019
    2120import javax.swing.BorderFactory;
     21import javax.swing.BoxLayout;
    2222import javax.swing.JCheckBox;
    2323import javax.swing.JEditorPane;
     
    7676    protected JPanel buildUploadCommentPanel() {
    7777        JPanel pnl = new JPanel(new GridBagLayout());
    78         pnl.setBorder(BorderFactory.createTitledBorder(tr("Tags of changeset {0}", "")));
    79 
    80         JEditorPane commentLabel = new JMultilineLabel("<html><b>" + tr("Provide a brief comment for the changes you are uploading:"));
    81         pnl.add(commentLabel, GBC.eol().insets(0, 5, 10, 3).fill(GBC.HORIZONTAL));
     78        pnl.setBorder(BorderFactory.createTitledBorder(tr("Provide a brief comment for the changes you are uploading:")));
     79
    8280        hcbUploadComment.setToolTipText(tr("Enter an upload comment"));
    8381        hcbUploadComment.setMaxTextLength(Changeset.MAX_CHANGESET_TAG_LENGTH);
     
    8785        hcbUploadComment.getEditorComponent().addFocusListener(commentModelListener);
    8886        pnl.add(hcbUploadComment, GBC.eol().fill(GBC.HORIZONTAL));
    89 
    90         JEditorPane sourceLabel = new JMultilineLabel("<html><b>" + tr("Specify the data source for the changes") + ":</b>");
    91         pnl.add(sourceLabel, GBC.eol().insets(0, 8, 10, 0).fill(GBC.HORIZONTAL));
     87        JLabel hcbUploadCommentFeedback = new JLabel();
     88        pnl.add(hcbUploadCommentFeedback, GBC.eol().insets(0, 3, 0, 0).fill(GBC.HORIZONTAL));
     89        new UploadTextComponentValidator.UploadCommentValidator(hcbUploadComment.getEditorComponent(), hcbUploadCommentFeedback);
     90        return pnl;
     91    }
     92
     93    protected JPanel buildUploadSourcePanel() {
     94        JPanel pnl = new JPanel(new GridBagLayout());
     95        pnl.setBorder(BorderFactory.createTitledBorder(tr("Specify the data source for the changes")));
     96
    9297        JEditorPane obtainSourceOnce = new JMultilineLabel(
    9398                "<html>(<a href=\"urn:changeset-source\">" + tr("just once") + "</a>)</html>");
     
    119124        hcbUploadSource.getEditorComponent().addFocusListener(sourceModelListener);
    120125        pnl.add(hcbUploadSource, GBC.eol().fill(GBC.HORIZONTAL));
     126        JLabel hcbUploadSourceFeedback = new JLabel();
     127        pnl.add(hcbUploadSourceFeedback, GBC.eol().insets(0, 3, 0, 0).fill(GBC.HORIZONTAL));
     128        new UploadTextComponentValidator.UploadSourceValidator(hcbUploadSource.getEditorComponent(), hcbUploadSourceFeedback);
    121129        if (obtainSourceAutomatically.isSelected()) {
    122130            automaticallyAddSource();
     
    181189
    182190    protected void build() {
    183         setLayout(new BorderLayout());
     191        setLayout(new BoxLayout(this, BoxLayout.Y_AXIS));
    184192        setBorder(BorderFactory.createEmptyBorder(3, 3, 3, 3));
    185         add(buildUploadCommentPanel(), BorderLayout.NORTH);
    186         add(pnlUploadParameterSummary, BorderLayout.CENTER);
     193        add(buildUploadCommentPanel());
     194        add(buildUploadSourcePanel());
     195        add(pnlUploadParameterSummary);
    187196        if (Config.getPref().getBoolean("upload.show.review.request", true)) {
    188             add(cbRequestReview, BorderLayout.SOUTH);
     197            JPanel pnl = new JPanel(new GridBagLayout());
     198            pnl.add(cbRequestReview, GBC.eol().fill(GBC.HORIZONTAL));
     199            add(pnl);
    189200            cbRequestReview.addItemListener(e -> changesetReviewModel.setReviewRequested(e.getStateChange() == ItemEvent.SELECTED));
    190201        }
  • trunk/src/org/openstreetmap/josm/gui/io/UploadDialog.java

    r16468 r16672  
    1717import java.lang.Character.UnicodeBlock;
    1818import java.util.ArrayList;
    19 import java.util.Arrays;
    2019import java.util.Collection;
    2120import java.util.Collections;
     
    3029import javax.swing.AbstractAction;
    3130import javax.swing.BorderFactory;
    32 import javax.swing.Icon;
    3331import javax.swing.JButton;
    3432import javax.swing.JOptionPane;
     
    4139import org.openstreetmap.josm.data.osm.DataSet;
    4240import org.openstreetmap.josm.data.osm.OsmPrimitive;
    43 import org.openstreetmap.josm.gui.ExtendedDialog;
    4441import org.openstreetmap.josm.gui.HelpAwareOptionPane;
    4542import org.openstreetmap.josm.gui.MainApplication;
     
    5754import org.openstreetmap.josm.spi.preferences.Setting;
    5855import org.openstreetmap.josm.tools.GBC;
    59 import org.openstreetmap.josm.tools.ImageOverlay;
    6056import org.openstreetmap.josm.tools.ImageProvider;
    61 import org.openstreetmap.josm.tools.ImageProvider.ImageSizes;
    6257import org.openstreetmap.josm.tools.InputMapUtils;
    6358import org.openstreetmap.josm.tools.Utils;
     
    450445        }
    451446
    452         /**
    453          * Displays a warning message indicating that the upload comment is empty/short.
    454          * @return true if the user wants to revisit, false if they want to continue
    455          */
    456         protected boolean warnUploadComment() {
    457             return warnUploadTag(
    458                     tr("Please revise upload comment"),
    459                     tr("Your upload comment is <i>empty</i>, or <i>very short</i>.<br /><br />" +
    460                             "This is technically allowed, but please consider that many users who are<br />" +
    461                             "watching changes in their area depend on meaningful changeset comments<br />" +
    462                             "to understand what is going on!<br /><br />" +
    463                             "If you spend a minute now to explain your change, you will make life<br />" +
    464                             "easier for many other mappers."),
    465                     "upload_comment_is_empty_or_very_short"
    466             );
    467         }
    468 
    469         /**
    470          * Displays a warning message indicating that no changeset source is given.
    471          * @return true if the user wants to revisit, false if they want to continue
    472          */
    473         protected boolean warnUploadSource() {
    474             return warnUploadTag(
    475                     tr("Please specify a changeset source"),
    476                     tr("You did not specify a source for your changes.<br />" +
    477                             "It is technically allowed, but this information helps<br />" +
    478                             "other users to understand the origins of the data.<br /><br />" +
    479                             "If you spend a minute now to explain your change, you will make life<br />" +
    480                             "easier for many other mappers."),
    481                     "upload_source_is_empty"
    482             );
    483         }
    484 
    485         /**
    486          * Displays a warning message indicating that the upload comment is rejected.
    487          * @param details details explaining why
    488          * @return {@code true}
    489          */
    490         protected boolean warnRejectedUploadComment(String details) {
    491             return warnRejectedUploadTag(
    492                     tr("Please revise upload comment"),
    493                     tr("Your upload comment is <i>rejected</i>.") + "<br />" + details
    494             );
    495         }
    496 
    497         /**
    498          * Displays a warning message indicating that the changeset source is rejected.
    499          * @param details details explaining why
    500          * @return {@code true}
    501          */
    502         protected boolean warnRejectedUploadSource(String details) {
    503             return warnRejectedUploadTag(
    504                     tr("Please revise changeset source"),
    505                     tr("Your changeset source is <i>rejected</i>.") + "<br />" + details
    506             );
    507         }
    508 
    509         /**
    510          * Warn about an upload tag with the possibility of resuming the upload.
    511          * @param title dialog title
    512          * @param message dialog message
    513          * @param togglePref preference entry to offer the user a "Do not show again" checkbox for the dialog
    514          * @return {@code true} if the user wants to revise the upload tag
    515          */
    516         protected boolean warnUploadTag(final String title, final String message, final String togglePref) {
    517             return warnUploadTag(title, message, togglePref, true);
    518         }
    519 
    520         /**
    521          * Warn about an upload tag without the possibility of resuming the upload.
    522          * @param title dialog title
    523          * @param message dialog message
    524          * @return {@code true}
    525          */
    526         protected boolean warnRejectedUploadTag(final String title, final String message) {
    527             return warnUploadTag(title, message, null, false);
    528         }
    529 
    530         private boolean warnUploadTag(final String title, final String message, final String togglePref, boolean allowContinue) {
    531             List<String> buttonTexts = new ArrayList<>(Arrays.asList(tr("Revise"), tr("Cancel")));
    532             List<Icon> buttonIcons = new ArrayList<>(Arrays.asList(
    533                     new ImageProvider("ok").setMaxSize(ImageSizes.LARGEICON).get(),
    534                     new ImageProvider("cancel").setMaxSize(ImageSizes.LARGEICON).get()));
    535             List<String> tooltips = new ArrayList<>(Arrays.asList(
    536                     tr("Return to the previous dialog to enter a more descriptive comment"),
    537                     tr("Cancel and return to the previous dialog")));
    538             if (allowContinue) {
    539                 buttonTexts.add(tr("Continue as is"));
    540                 buttonIcons.add(new ImageProvider("upload").setMaxSize(ImageSizes.LARGEICON).addOverlay(
    541                         new ImageOverlay(new ImageProvider("warning-small"), 0.5, 0.5, 1.0, 1.0)).get());
    542                 tooltips.add(tr("Ignore this hint and upload anyway"));
    543             }
    544 
    545             ExtendedDialog dlg = new ExtendedDialog((Component) dialog, title, buttonTexts.toArray(new String[] {})) {
    546                 @Override
    547                 public void setupDialog() {
    548                     super.setupDialog();
    549                     InputMapUtils.addCtrlEnterAction(getRootPane(), buttons.get(buttons.size() - 1).getAction());
    550                 }
    551             };
    552             dlg.setContent("<html>" + message + "</html>");
    553             dlg.setButtonIcons(buttonIcons.toArray(new Icon[] {}));
    554             dlg.setToolTipTexts(tooltips.toArray(new String[] {}));
    555             dlg.setIcon(JOptionPane.WARNING_MESSAGE);
    556             if (allowContinue) {
    557                 dlg.toggleEnable(togglePref);
    558             }
    559             dlg.setCancelButton(1, 2);
    560             return dlg.showDialog().getValue() != 3;
    561         }
    562 
    563447        protected void warnIllegalChunkSize() {
    564448            HelpAwareOptionPane.showOptionDialog(
     
    614498            // force update of model in case dialog is closed before focus lost event, see #17452
    615499            dialog.forceUpdateActiveField();
    616 
    617             final List<String> def = Collections.emptyList();
    618             final String uploadComment = dialog.getUploadComment();
    619             final String uploadCommentRejection = validateUploadTag(
    620                     uploadComment, "upload.comment", def, def, def);
    621             if ((isUploadCommentTooShort(uploadComment) && warnUploadComment()) ||
    622                 (uploadCommentRejection != null && warnRejectedUploadComment(uploadCommentRejection))) {
    623                 // abort for missing or rejected comment
    624                 dialog.handleMissingComment();
    625                 return;
    626             }
    627             final String uploadSource = dialog.getUploadSource();
    628             final String uploadSourceRejection = validateUploadTag(
    629                     uploadSource, "upload.source", def, def, def);
    630             if ((Utils.isStripEmpty(uploadSource) && warnUploadSource()) ||
    631                     (uploadSourceRejection != null && warnRejectedUploadSource(uploadSourceRejection))) {
    632                 // abort for missing or rejected changeset source
    633                 dialog.handleMissingSource();
    634                 return;
    635             }
    636500
    637501            /* test for empty tags in the changeset metadata and proceed only after user's confirmation.
  • trunk/src/org/openstreetmap/josm/gui/widgets/AbstractTextComponentValidator.java

    r16553 r16672  
    3333 */
    3434public abstract class AbstractTextComponentValidator implements ActionListener, FocusListener, DocumentListener, PropertyChangeListener {
    35     private static final Border ERROR_BORDER = BorderFactory.createLineBorder(Color.RED, 1);
    36     private static final Color ERROR_BACKGROUND = new Color(255, 224, 224);
    37 
    38     private JTextComponent tc;
    39     /** remembers whether the content of the text component is currently valid or not; null means,
    40      * we don't know yet
    41      */
    42     private Boolean valid;
     35    protected static final Color ERROR_COLOR = Color.RED;
     36    protected static final Border ERROR_BORDER = BorderFactory.createLineBorder(ERROR_COLOR, 1);
     37    protected static final Color ERROR_BACKGROUND = new Color(0xFFCCCC);
     38    protected static final Color WARNING_COLOR = new Color(0xFFA500);
     39    protected static final Border WARNING_BORDER = BorderFactory.createLineBorder(WARNING_COLOR, 1);
     40    protected static final Color WARNING_BACKGROUND = new Color(0xFFEDCC);
     41    protected static final Color VALID_COLOR = new Color(0x008000);
     42    protected static final Border VALID_BORDER = BorderFactory.createLineBorder(VALID_COLOR, 1);
     43
     44    private final JTextComponent tc;
     45    // remembers whether the content of the text component is currently valid or not; null means, we don't know yet
     46    private Status status;
    4347    // remember the message
    4448    private String msg;
    4549
     50    enum Status {
     51        INVALID, WARNING, VALID
     52    }
     53
    4654    protected void feedbackInvalid(String msg) {
    47         if (valid == null || valid || !Objects.equals(msg, this.msg)) {
     55        if (hasChanged(msg, Status.INVALID)) {
    4856            // only provide feedback if the validity has changed. This avoids unnecessary UI updates.
    4957            tc.setBorder(ERROR_BORDER);
    5058            tc.setBackground(ERROR_BACKGROUND);
    5159            tc.setToolTipText(msg);
    52             valid = Boolean.FALSE;
     60            this.status = Status.INVALID;
     61            this.msg = msg;
     62        }
     63    }
     64
     65    protected void feedbackWarning(String msg) {
     66        if (hasChanged(msg, Status.WARNING)) {
     67            // only provide feedback if the validity has changed. This avoids unnecessary UI updates.
     68            tc.setBorder(WARNING_BORDER);
     69            tc.setBackground(WARNING_BACKGROUND);
     70            tc.setToolTipText(msg);
     71            this.status = Status.WARNING;
    5372            this.msg = msg;
    5473        }
     
    6079
    6180    protected void feedbackValid(String msg) {
    62         if (valid == null || !valid || !Objects.equals(msg, this.msg)) {
     81        if (hasChanged(msg, Status.VALID)) {
    6382            // only provide feedback if the validity has changed. This avoids unnecessary UI updates.
    64             tc.setBorder(UIManager.getBorder("TextField.border"));
     83            tc.setBorder(VALID_BORDER);
    6584            tc.setBackground(UIManager.getColor("TextField.background"));
    6685            tc.setToolTipText(msg == null ? "" : msg);
    67             valid = Boolean.TRUE;
     86            this.status = Status.VALID;
    6887            this.msg = msg;
    6988        }
     89    }
     90
     91    private boolean hasChanged(String msg, Status status) {
     92        return !(Objects.equals(status, this.status) && Objects.equals(msg, this.msg));
    7093    }
    7194
  • trunk/test/unit/org/openstreetmap/josm/gui/io/UploadDialogTest.java

    r16160 r16672  
    1515import java.util.function.Supplier;
    1616
    17 import javax.swing.JButton;
    1817import javax.swing.JOptionPane;
    1918
     
    2120import org.junit.Test;
    2221import org.openstreetmap.josm.TestUtils;
    23 import org.openstreetmap.josm.gui.ExtendedDialog;
    2422import org.openstreetmap.josm.gui.io.UploadDialog.UploadAction;
    2523import org.openstreetmap.josm.io.UploadStrategySpecification;
    2624import org.openstreetmap.josm.spi.preferences.Config;
    2725import org.openstreetmap.josm.testutils.JOSMTestRules;
    28 import org.openstreetmap.josm.testutils.mockers.ExtendedDialogMocker;
    2926import org.openstreetmap.josm.testutils.mockers.WindowMocker;
    3027
    3128import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
    32 import mockit.Invocation;
    33 import mockit.Mock;
    3429
    3530/**
     
    119114        MockUploadDialog uploadDialog = new MockUploadDialog(null, null);
    120115        new UploadDialog.CancelAction(uploadDialog).actionPerformed(null);
    121     }
    122 
    123     /**
    124      * Test of {@link UploadDialog.UploadAction} class.
    125      */
    126     @Test
    127     public void testUploadAction() {
    128         TestUtils.assumeWorkingJMockit();
    129         ExtendedDialogMocker edMocker = new ExtendedDialogMocker() {
    130             @Mock
    131             void setupDialog(Invocation invocation) throws Exception {
    132                 if (GraphicsEnvironment.isHeadless()) {
    133                     final int nButtons = ((String[]) TestUtils.getPrivateField(
    134                             ExtendedDialog.class, invocation.getInvokedInstance(), "bTexts")).length;
    135                     @SuppressWarnings("unchecked")
    136                     final List<JButton> buttons = (List<JButton>) TestUtils.getPrivateField(
    137                             ExtendedDialog.class, invocation.getInvokedInstance(), "buttons");
    138 
    139                     for (int i = 0; i < nButtons; i++) {
    140                         buttons.add(new JButton());
    141                     }
    142                 } else {
    143                     invocation.proceed();
    144                 }
    145             }
    146         };
    147         edMocker.getMockResultMap().put("<html>Your upload comment is <i>empty</i>, or <i>very short</i>.<br /><br />This is "
    148                 + "technically allowed, but please consider that many users who are<br />watching changes "
    149                 + "in their area depend on meaningful changeset comments<br />to understand what is going "
    150                 + "on!<br /><br />If you spend a minute now to explain your change, you will make life<br />"
    151                 + "easier for many other mappers.</html>", "Revise");
    152         edMocker.getMockResultMap().put("<html>You did not specify a source for your changes.<br />It is technically allowed, "
    153                 + "but this information helps<br />other users to understand the origins of the data."
    154                 + "<br /><br />If you spend a minute now to explain your change, you will make life"
    155                 + "<br />easier for many other mappers.</html>", "Revise");
    156 
    157         MockUploadDialog uploadDialog = new MockUploadDialog("comment", "source");
    158         new UploadDialog.UploadAction(uploadDialog).actionPerformed(null);
    159 
    160         assertEquals(1, uploadDialog.handleMissingCommentCalls);
    161         assertEquals(0, uploadDialog.handleMissingSourceCalls);
    162         assertEquals(1, edMocker.getInvocationLog().size());
    163         Object[] invocationLogEntry = edMocker.getInvocationLog().get(0);
    164         assertEquals(1, (int) invocationLogEntry[0]);
    165         assertEquals("Please revise upload comment", invocationLogEntry[2]);
    166         edMocker.resetInvocationLog();
    167 
    168         uploadDialog = new MockUploadDialog("", "source");
    169         new UploadDialog.UploadAction(uploadDialog).actionPerformed(null);
    170 
    171         assertEquals(1, uploadDialog.handleMissingCommentCalls);
    172         assertEquals(0, uploadDialog.handleMissingSourceCalls);
    173         assertEquals(1, edMocker.getInvocationLog().size());
    174         invocationLogEntry = edMocker.getInvocationLog().get(0);
    175         assertEquals(1, (int) invocationLogEntry[0]);
    176         assertEquals("Please revise upload comment", invocationLogEntry[2]);
    177         edMocker.resetInvocationLog();
    178 
    179         uploadDialog = new MockUploadDialog("comment", "");
    180         new UploadDialog.UploadAction(uploadDialog).actionPerformed(null);
    181 
    182         assertEquals(1, uploadDialog.handleMissingCommentCalls);
    183         assertEquals(0, uploadDialog.handleMissingSourceCalls);
    184         assertEquals(1, edMocker.getInvocationLog().size());
    185         invocationLogEntry = edMocker.getInvocationLog().get(0);
    186         assertEquals(1, (int) invocationLogEntry[0]);
    187         assertEquals("Please revise upload comment", invocationLogEntry[2]);
    188         edMocker.resetInvocationLog();
    189 
    190         uploadDialog = new MockUploadDialog("a comment long enough", "");
    191         new UploadDialog.UploadAction(uploadDialog).actionPerformed(null);
    192 
    193         assertEquals(0, uploadDialog.handleMissingCommentCalls);
    194         assertEquals(1, uploadDialog.handleMissingSourceCalls);
    195         assertEquals(1, edMocker.getInvocationLog().size());
    196         invocationLogEntry = edMocker.getInvocationLog().get(0);
    197         assertEquals(1, (int) invocationLogEntry[0]);
    198         assertEquals("Please specify a changeset source", invocationLogEntry[2]);
    199         edMocker.resetInvocationLog();
    200 
    201         uploadDialog = new MockUploadDialog("a comment long enough", "a source long enough");
    202         new UploadDialog.UploadAction(uploadDialog).actionPerformed(null);
    203 
    204         assertEquals(0, uploadDialog.handleMissingCommentCalls);
    205         assertEquals(0, uploadDialog.handleMissingSourceCalls);
    206         assertEquals(0, edMocker.getInvocationLog().size());
    207116    }
    208117
Note: See TracChangeset for help on using the changeset viewer.