Index: src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetItemGuiSupport.java =================================================================== --- src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetItemGuiSupport.java (revision 18252) +++ src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetItemGuiSupport.java (working copy) @@ -24,6 +24,7 @@ public final class TaggingPresetItemGuiSupport implements TemplateEngineDataProvider { private final Collection selected; + /** True if all selected primitives matched this preset at the moment it was openend. */ private final boolean presetInitiallyMatches; private final Supplier> changedTagsSupplier; private final ListenerList listeners = ListenerList.create(); @@ -83,9 +84,9 @@ } /** - * Returns whether the preset initially matched (before opening the dialog) + * Returns true if all selected primitives matched this preset (before opening the dialog) * - * @return whether the preset initially matched + * @return true if the preset initially matched */ public boolean isPresetInitiallyMatches() { return presetInitiallyMatches; Index: src/org/openstreetmap/josm/gui/tagging/presets/items/Check.java =================================================================== --- src/org/openstreetmap/josm/gui/tagging/presets/items/Check.java (revision 18252) +++ src/org/openstreetmap/josm/gui/tagging/presets/items/Check.java (working copy) @@ -39,12 +39,12 @@ // find out if our key is already used in the selection. final Usage usage = determineBooleanUsage(support.getSelected(), key); - final String oneValue = usage.values.isEmpty() ? null : usage.values.last(); + final String oneValue = usage.map.isEmpty() ? null : usage.map.lastKey(); def = "on".equals(default_) ? Boolean.TRUE : "off".equals(default_) ? Boolean.FALSE : null; initializeLocaleText(null); - if (usage.values.size() < 2 && (oneValue == null || value_on.equals(oneValue) || value_off.equals(oneValue))) { + if (usage.map.size() < 2 && (oneValue == null || value_on.equals(oneValue) || value_off.equals(oneValue))) { if (def != null && !PROP_FILL_DEFAULT.get()) { // default is set and filling default values feature is disabled - check if all primitives are untagged for (OsmPrimitive s : support.getSelected()) { Index: src/org/openstreetmap/josm/gui/tagging/presets/items/Combo.java =================================================================== --- src/org/openstreetmap/josm/gui/tagging/presets/items/Combo.java (revision 18252) +++ src/org/openstreetmap/josm/gui/tagging/presets/items/Combo.java (working copy) @@ -12,11 +12,13 @@ import java.awt.event.ComponentEvent; import java.util.Arrays; import java.util.Comparator; +import java.util.TreeMap; import javax.swing.AbstractAction; import javax.swing.JButton; import javax.swing.JColorChooser; import javax.swing.JComponent; +import javax.swing.JLabel; import javax.swing.JPanel; import org.openstreetmap.josm.data.tagging.ac.AutoCompletionItem; @@ -29,6 +31,7 @@ import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetItem; import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetItemGuiSupport; import org.openstreetmap.josm.gui.widgets.JosmComboBox; +import org.openstreetmap.josm.gui.widgets.OrientationAction; import org.openstreetmap.josm.tools.ColorHelper; import org.openstreetmap.josm.tools.GBC; @@ -74,21 +77,36 @@ delimiter = ','; } + private void addEntry(PresetListEntry entry) { + if (!seenValues.containsKey(entry.value)) { + dropDownModel.addElement(entry); + seenValues.put(entry.value, entry); + } + } + @Override - protected JComponent addToPanelAnchor(JPanel p, String def, TaggingPresetItemGuiSupport support) { - if (!usage.unused()) { - for (String s : usage.values) { - presetListEntries.add(new PresetListEntry(s)); - } + protected boolean addToPanel(JPanel p, TaggingPresetItemGuiSupport support) { + initializeLocaleText(null); + usage = determineTextUsage(support.getSelected(), key); + seenValues = new TreeMap<>(); + // get the standard values from the preset definition + initListEntries(); + + // init the model + dropDownModel = new AutoCompComboBoxModel(Comparator.naturalOrder()); + + if (!usage.hasUniqueValue() && !usage.unused()) { + addEntry(PresetListEntry.ENTRY_DIFFERENT); } - if (def != null) { - presetListEntries.add(new PresetListEntry(def)); + presetListEntries.forEach(this::addEntry); + if (default_ != null) { + addEntry(new PresetListEntry(default_, this)); } - presetListEntries.add(new PresetListEntry("")); + addEntry(PresetListEntry.ENTRY_EMPTY); - dropDownModel = new AutoCompComboBoxModel(Comparator.naturalOrder()); - autoCompModel = new AutoCompComboBoxModel(Comparator.naturalOrder()); - presetListEntries.forEach(dropDownModel::addElement); + usage.map.forEach((value, count) -> { + addEntry(new PresetListEntry(value, this)); + }); combobox = new JosmComboBox<>(dropDownModel); AutoCompComboBoxEditor editor = new AutoCompComboBoxEditor<>(); @@ -97,7 +115,7 @@ // The default behaviour of JComboBox is to size the editor according to the tallest item in // the dropdown list. We don't want that to happen because we want to show taller items in // the list than in the editor. We can't use - // {@code combobox.setPrototypeDisplayValue(new PresetListEntry(" "));} because that would + // {@code combobox.setPrototypeDisplayValue(PresetListEntry.ENTRY_EMPTY);} because that would // set a fixed cell height in JList. combobox.setPreferredHeight(combobox.getPreferredSize().height); @@ -106,6 +124,7 @@ combobox.setRenderer(new ComboMultiSelectListCellRenderer(combobox, combobox.getRenderer(), 200, key)); combobox.setEditable(editable); + autoCompModel = new AutoCompComboBoxModel(Comparator.naturalOrder()); getAllForKeys(Arrays.asList(key)).forEach(autoCompModel::addElement); getDisplayValues().forEach(s -> autoCompModel.addElement(new AutoCompletionItem(s, AutoCompletionPriority.IS_IN_STANDARD))); @@ -119,27 +138,53 @@ tf.setMaxTextLength(length); } + JLabel label = addLabel(p); + if (key != null && ("colour".equals(key) || key.startsWith("colour:") || key.endsWith(":colour"))) { - p.add(combobox, GBC.std().fill(GBC.HORIZONTAL)); + p.add(combobox, GBC.std().fill(GBC.HORIZONTAL)); // NOSONAR JButton button = new JButton(new ChooseColorAction()); button.setOpaque(true); button.setBorderPainted(false); button.setCursor(Cursor.getPredefinedCursor(Cursor.HAND_CURSOR)); - p.add(button, GBC.eol().fill(GBC.VERTICAL)); + p.add(button, GBC.eol().fill(GBC.VERTICAL)); // NOSONAR ActionListener updateColor = ignore -> button.setBackground(getColor()); updateColor.actionPerformed(null); combobox.addActionListener(updateColor); } else { - p.add(combobox, GBC.eol().fill(GBC.HORIZONTAL)); + p.add(combobox, GBC.eol().fill(GBC.HORIZONTAL)); // NOSONAR } - Object itemToSelect = getItemToSelect(default_, support, false); - combobox.setSelectedItemText(itemToSelect == null ? null : itemToSelect.toString()); - combobox.addActionListener(l -> support.fireItemValueModified(this, key, getSelectedValue())); + String valueToSelect = getInitialValue(default_); + if (valueToSelect != null) { + PresetListEntry selItem = find(valueToSelect); + if (selItem != null) { + combobox.setSelectedItem(selItem); + } else { + combobox.setText(valueToSelect); + } + } + combobox.addActionListener(l -> support.fireItemValueModified(this, key, getSelectedItem().value)); combobox.addComponentListener(new ComponentListener()); - return combobox; + + label.setLabelFor(combobox); + combobox.setToolTipText(getKeyTooltipText()); + combobox.applyComponentOrientation(OrientationAction.getValueOrientation(key)); + + return true; } + /** + * Finds the PresetListEntry that matches value. + *

+ * Looks in the model for an element whose {@code value} matches {@code value}. + * + * @param value The value to match. + * @return The entry or null + */ + private PresetListEntry find(String value) { + return dropDownModel.asCollection().stream().filter(o -> o.value.equals(value)).findAny().orElse(null); + } + class ChooseColorAction extends AbstractAction { ChooseColorAction() { putValue(SHORT_DESCRIPTION, tr("Choose a color")); @@ -160,7 +205,7 @@ } protected Color getColor() { - String colorString = String.valueOf(getSelectedValue()); + String colorString = getSelectedItem().value; return colorString.startsWith("#") ? ColorHelper.html2color(colorString) : CSSColors.get(colorString); @@ -167,15 +212,20 @@ } @Override - protected Object getSelectedItem() { - return combobox.getSelectedItem(); + protected PresetListEntry getSelectedItem() { + Object sel = combobox.getSelectedItem(); + if (sel instanceof PresetListEntry) + // selected from the dropdown + return (PresetListEntry) sel; + if (sel instanceof String) { + // free edit. If the free edit corresponds to a known entry, use that entry. This is + // to avoid that we write a display_value to the tag's value, eg. if the user did an + // undo. + PresetListEntry selItem = dropDownModel.find((String) sel); + if (selItem != null) + return selItem; + return new PresetListEntry((String) sel, this); + } + return PresetListEntry.ENTRY_EMPTY; } - - @Override - protected String getDisplayIfNull() { - if (combobox.isEditable()) - return combobox.getEditor().getItem().toString(); - else - return null; - } } Index: src/org/openstreetmap/josm/gui/tagging/presets/items/ComboMultiSelect.java =================================================================== --- src/org/openstreetmap/josm/gui/tagging/presets/items/ComboMultiSelect.java (revision 18252) +++ src/org/openstreetmap/josm/gui/tagging/presets/items/ComboMultiSelect.java (working copy) @@ -2,7 +2,6 @@ package org.openstreetmap.josm.gui.tagging.presets.items; import static org.openstreetmap.josm.tools.I18n.tr; -import static org.openstreetmap.josm.tools.I18n.trc; import java.awt.Component; import java.lang.reflect.Method; @@ -12,12 +11,10 @@ import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Objects; -import java.util.Set; -import java.util.concurrent.CopyOnWriteArraySet; +import java.util.Map; +import java.util.TreeMap; import java.util.stream.Collectors; -import javax.swing.JComponent; import javax.swing.JLabel; import javax.swing.JList; import javax.swing.JPanel; @@ -24,14 +21,13 @@ import javax.swing.ListCellRenderer; import org.openstreetmap.josm.data.osm.Tag; -import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetItemGuiSupport; import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetSelector; import org.openstreetmap.josm.gui.tagging.presets.TaggingPresets; import org.openstreetmap.josm.gui.widgets.JosmListCellRenderer; import org.openstreetmap.josm.gui.widgets.OrientationAction; +import org.openstreetmap.josm.tools.AlphanumComparator; import org.openstreetmap.josm.tools.GBC; import org.openstreetmap.josm.tools.Logging; -import org.openstreetmap.josm.tools.Utils; /** * Abstract superclass for combo box and multi-select list types. @@ -88,10 +84,16 @@ /** whether to use values for search via {@link TaggingPresetSelector} */ public boolean values_searchable; // NOSONAR - protected final Set presetListEntries = new CopyOnWriteArraySet<>(); - private boolean initialized; + /** + * The standard entries in the combobox dropdown or multiselect list. These entries are defined + * in {@code defaultpresets.xml} (or in other custom preset files). + */ + protected final List presetListEntries = new ArrayList<>(); + /** Helps avoid duplicate list entries */ + protected Map seenValues = new TreeMap<>(); protected Usage usage; - protected Object originalValue; + /** Used to see if the user edited the value. May be null. */ + protected String originalValue; /** * A list cell renderer that paints a short text in the current value pane and and a longer text @@ -133,12 +135,7 @@ // editor-ersatz of a readonly combobox. fixes #6157 l.setText(value.getListDisplay(width)); } - String tt = value.value; - if (tt != null && !tt.isEmpty()) { - l.setToolTipText(tr("Sets the key ''{0}'' to the value ''{1}''.", key, tt)); - } else { - l.setToolTipText(tr("Clears the key ''{0}''.", key)); - } + l.setToolTipText(value.getToolTipText(key)); l.setIcon(value.getIcon()); return l; } @@ -152,9 +149,10 @@ * @param s the string * @return splitted items */ - public static String[] splitEscaped(char delimiter, String s) { + public static List splitEscaped(char delimiter, String s) { if (s == null) - return new String[0]; + return null; // NOSONAR + List result = new ArrayList<>(); boolean backslash = false; StringBuilder item = new StringBuilder(); @@ -175,16 +173,19 @@ if (item.length() > 0) { result.add(item.toString()); } - return result.toArray(new String[0]); + return result; } - protected abstract Object getSelectedItem(); + /** + * Returns the value selected in the combobox or a synthetic value if a multiselect. + * + * @return the value + */ + protected abstract PresetListEntry getSelectedItem(); - protected abstract JComponent addToPanelAnchor(JPanel p, String def, TaggingPresetItemGuiSupport support); - @Override public Collection getValues() { - initListEntries(false); + initListEntries(); return presetListEntries.stream().map(x -> x.value).collect(Collectors.toSet()); } @@ -193,20 +194,17 @@ * @return the values to display */ public Collection getDisplayValues() { - initListEntries(false); + initListEntries(); return presetListEntries.stream().map(PresetListEntry::getDisplayValue).collect(Collectors.toList()); } - @Override - public boolean addToPanel(JPanel p, TaggingPresetItemGuiSupport support) { - initListEntries(true); - - // find out if our key is already used in the selection. - usage = determineTextUsage(support.getSelected(), key); - if (!usage.hasUniqueValue() && !usage.unused()) { - presetListEntries.add(new PresetListEntry(DIFFERENT)); - } - + /** + * Adds the label to the panel + * + * @param p the panel + * @return the label + */ + protected JLabel addLabel(JPanel p) { final JLabel label = new JLabel(tr("{0}:", locale_text)); addIcon(label); label.setToolTipText(getKeyTooltipText()); @@ -213,196 +211,150 @@ label.setComponentPopupMenu(getPopupMenu()); label.applyComponentOrientation(OrientationAction.getDefaultComponentOrientation()); p.add(label, GBC.std().insets(0, 0, 10, 0)); - JComponent component = addToPanelAnchor(p, default_, support); - label.setLabelFor(component); - component.setToolTipText(getKeyTooltipText()); - component.applyComponentOrientation(OrientationAction.getValueOrientation(key)); + return label; + } - return true; + protected void initListEntries() { + if (presetListEntries.isEmpty()) { + initListEntriesFromAttributes(); + } } - private void initListEntries(boolean cleanup) { - if (initialized) { - if (cleanup) { // do not cleanup for #getDisplayValues used in Combo#addToPanelAnchor - presetListEntries.remove(new PresetListEntry(DIFFERENT)); // possibly added in #addToPanel - } - return; - } else if (presetListEntries.isEmpty()) { - initListEntriesFromAttributes(); - } else { - if (values != null) { - Logging.warn(tr("Warning in tagging preset \"{0}-{1}\": " - + "Ignoring ''{2}'' attribute as ''{3}'' elements are given.", - key, text, "values", "list_entry")); - } - if (display_values != null || locale_display_values != null) { - Logging.warn(tr("Warning in tagging preset \"{0}-{1}\": " - + "Ignoring ''{2}'' attribute as ''{3}'' elements are given.", - key, text, "display_values", "list_entry")); - } - if (short_descriptions != null || locale_short_descriptions != null) { - Logging.warn(tr("Warning in tagging preset \"{0}-{1}\": " - + "Ignoring ''{2}'' attribute as ''{3}'' elements are given.", - key, text, "short_descriptions", "list_entry")); - } - for (PresetListEntry e : presetListEntries) { - if (e.value_context == null) { - e.value_context = values_context; + private List getValuesFromCode(String values_from) { + // get the values from a Java function + String[] classMethod = values_from.split("#", -1); + if (classMethod.length == 2) { + try { + Method method = Class.forName(classMethod[0]).getMethod(classMethod[1]); + // Check method is public static String[] methodName() + int mod = method.getModifiers(); + if (Modifier.isPublic(mod) && Modifier.isStatic(mod) + && method.getReturnType().equals(String[].class) && method.getParameterTypes().length == 0) { + return Arrays.asList((String[]) method.invoke(null)); + } else { + Logging.error(tr("Broken tagging preset \"{0}-{1}\" - Java method given in ''values_from'' is not \"{2}\"", key, text, + "public static String[] methodName()")); } + } catch (ReflectiveOperationException e) { + Logging.error(tr("Broken tagging preset \"{0}-{1}\" - Java method given in ''values_from'' threw {2} ({3})", key, text, + e.getClass().getName(), e.getMessage())); + Logging.debug(e); } } - initializeLocaleText(null); - initialized = true; + return null; // NOSONAR } - private void initListEntriesFromAttributes() { + /** + * Checks if list {@code a} is either null or the same length as list {@code b}. + * + * @param a The list to check + * @param b The other list + * @param name The name of the list for error reporting + * @return {@code a} if both lists have the same length or {@code null} + */ + private List checkListsSameLength(List a, List b, String name) { + if (a != null && a.size() != b.size()) { + Logging.error(tr("Broken tagging preset \"{0}-{1}\" - number of items in ''{2}List'' must be the same as in ''values''", + key, text, name)); + Logging.error(tr("Detailed information: {0} <> {1}", a, b)); + return null; // NOSONAR + } + return a; + } - String[] valueArray = null; + protected void initListEntriesFromAttributes() { + List valueList = null; + List displayList = null; + List localeDisplayList = null; if (values_from != null) { - String[] classMethod = values_from.split("#", -1); - if (classMethod.length == 2) { - try { - Method method = Class.forName(classMethod[0]).getMethod(classMethod[1]); - // Check method is public static String[] methodName() - int mod = method.getModifiers(); - if (Modifier.isPublic(mod) && Modifier.isStatic(mod) - && method.getReturnType().equals(String[].class) && method.getParameterTypes().length == 0) { - valueArray = (String[]) method.invoke(null); - } else { - Logging.error(tr("Broken tagging preset \"{0}-{1}\" - Java method given in ''values_from'' is not \"{2}\"", key, text, - "public static String[] methodName()")); - } - } catch (ReflectiveOperationException e) { - Logging.error(tr("Broken tagging preset \"{0}-{1}\" - Java method given in ''values_from'' threw {2} ({3})", key, text, - e.getClass().getName(), e.getMessage())); - Logging.debug(e); - } - } + valueList = getValuesFromCode(values_from); } - if (valueArray == null) { - valueArray = splitEscaped(delimiter, values); - values = null; + if (valueList == null) { + // get from {@code values} attribute + valueList = splitEscaped(delimiter, values); } + if (valueList == null) { + return; + } - String[] displayArray = valueArray; if (!values_no_i18n) { - final String displ = Utils.firstNonNull(locale_display_values, display_values); - displayArray = displ == null ? valueArray : splitEscaped(delimiter, displ); + localeDisplayList = splitEscaped(delimiter, locale_display_values); + displayList = splitEscaped(delimiter, display_values); } + List localeShortDescriptionsList = splitEscaped(delimiter, locale_short_descriptions); + List shortDescriptionsList = splitEscaped(delimiter, short_descriptions); - final String descr = Utils.firstNonNull(locale_short_descriptions, short_descriptions); - String[] shortDescriptionsArray = descr == null ? null : splitEscaped(delimiter, descr); + displayList = checkListsSameLength(displayList, valueList, "display"); + localeDisplayList = checkListsSameLength(localeDisplayList, valueList, "localeDisplay"); + shortDescriptionsList = checkListsSameLength(shortDescriptionsList, valueList, "shortDescriptions"); + localeShortDescriptionsList = checkListsSameLength(localeShortDescriptionsList, valueList, "localeShortDescriptions"); - if (displayArray.length != valueArray.length) { - Logging.error(tr("Broken tagging preset \"{0}-{1}\" - number of items in ''display_values'' must be the same as in ''values''", - key, text)); - Logging.error(tr("Detailed information: {0} <> {1}", Arrays.toString(displayArray), Arrays.toString(valueArray))); - displayArray = valueArray; + for (int i = 0; i < valueList.size(); i++) { + final PresetListEntry e = new PresetListEntry(valueList.get(i), this); + if (displayList != null) + e.display_value = displayList.get(i); + if (localeDisplayList != null) + e.locale_display_value = localeDisplayList.get(i); + if (shortDescriptionsList != null) + e.short_description = shortDescriptionsList.get(i); + if (localeShortDescriptionsList != null) + e.locale_short_description = localeShortDescriptionsList.get(i); + addListEntry(e); } - if (shortDescriptionsArray != null && shortDescriptionsArray.length != valueArray.length) { - Logging.error(tr("Broken tagging preset \"{0}-{1}\" - number of items in ''short_descriptions'' must be the same as in ''values''", - key, text)); - Logging.error(tr("Detailed information: {0} <> {1}", Arrays.toString(shortDescriptionsArray), Arrays.toString(valueArray))); - shortDescriptionsArray = null; - } - - final List entries = new ArrayList<>(valueArray.length); - for (int i = 0; i < valueArray.length; i++) { - final PresetListEntry e = new PresetListEntry(valueArray[i]); - final String value = locale_display_values != null || values_no_i18n - ? displayArray[i] - : trc(values_context, fixPresetString(displayArray[i])); - e.locale_display_value = value == null ? null : value.intern(); - if (shortDescriptionsArray != null) { - final String description = locale_short_descriptions != null - ? shortDescriptionsArray[i] - : tr(fixPresetString(shortDescriptionsArray[i])); - e.locale_short_description = description == null ? null : description.intern(); - } - - entries.add(e); - } - if (values_sort && TaggingPresets.SORT_MENU.get()) { - Collections.sort(entries); + Collections.sort(presetListEntries, (a, b) -> AlphanumComparator.getInstance().compare(a.getDisplayValue(), b.getDisplayValue())); } - - addListEntries(entries); } - protected String getDisplayIfNull() { - return null; - } + /** + * Returns the initial value to use for this preset. + *

+ * The initial value is the value shown in the control when the preset dialogs opens. + * + * @param def The default value + * @return The initial value to use. + */ + protected String getInitialValue(String def) { + String initialValue = null; - protected Object getItemToSelect(String def, TaggingPresetItemGuiSupport support, boolean multi) { - final Object itemToSelect; if (usage.hasUniqueValue()) { - // all items have the same value (and there were no unset items) - originalValue = multi ? usage.getFirst() : getListEntry(usage.getFirst()); - itemToSelect = originalValue; - } else if (def != null && usage.unused()) { - // default is set and all items were unset - if (!usage.hadKeys() || PROP_FILL_DEFAULT.get() || isForceUseLastAsDefault()) { - // selected osm primitives are untagged or filling default feature is enabled - if (multi) { - itemToSelect = def; - } else { - PresetListEntry entry = getListEntry(def); - itemToSelect = entry == null ? "" : entry.getDisplayValue(); - } - } else { - // selected osm primitives are tagged and filling default feature is disabled - itemToSelect = ""; - } - originalValue = multi ? DIFFERENT : getListEntry(DIFFERENT); - } else if (usage.unused()) { - // all items were unset (and so is default) - originalValue = multi ? null : getListEntry(""); - if (LAST_VALUES.containsKey(key) && isUseLastAsDefault() && (!support.isPresetInitiallyMatches() || isForceUseLastAsDefault())) { - itemToSelect = getListEntry(LAST_VALUES.get(key)); - } else { - itemToSelect = originalValue; - } - } else { - originalValue = multi ? DIFFERENT : getListEntry(DIFFERENT); - itemToSelect = originalValue; + // all selected primitives have the same not empty value for this key + initialValue = usage.getFirst(); + } else if (!usage.unused()) { + // at least one primitive has a value for this key (but not all have the same one) + initialValue = DIFFERENT; + } else if (PROP_FILL_DEFAULT.get() || isForceUseLastAsDefault()) { + // at this point no primitive had any value for this key + // use the last value no matter what + initialValue = LAST_VALUES.get(key); + } else if (!usage.hadKeys() && isUseLastAsDefault()) { + // use the last value only on objects with no keys at all + initialValue = LAST_VALUES.get(key); + } else if (!usage.hadKeys()) { + // use the default only on objects with no keys at all + initialValue = def; } - return itemToSelect; + originalValue = initialValue; + return initialValue; } - protected String getSelectedValue() { - Object obj = getSelectedItem(); - String display = obj == null ? getDisplayIfNull() : obj.toString(); - - if (display == null) { - return ""; - } - return presetListEntries.stream() - .filter(entry -> Objects.equals(entry.toString(), display)) - .findFirst() - .map(entry -> entry.value) - .map(Utils::removeWhiteSpaces) - .orElse(display); - } - @Override public void addCommands(List changedTags) { - String value = getSelectedValue(); + String value = getSelectedItem().value; // no change if same as before - if (originalValue == null) { - if (value.isEmpty()) - return; - } else if (value.equals(originalValue.toString())) + if (value.isEmpty() && originalValue == null) return; + if (value.equals(originalValue)) + return; + changedTags.add(new Tag(key, value)); if (isUseLastAsDefault()) { LAST_VALUES.put(key, value); } - changedTags.add(new Tag(key, value)); } /** @@ -419,10 +371,25 @@ } } + /** + * Returns true if the last entered value should be used as default. + *

+ * Note: never used in {@code defaultpresets.xml}. + * + * @return true if the last entered value should be used as default. + */ protected boolean isUseLastAsDefault() { return use_last_as_default > 0; } + /** + * Returns true if the last entered value should be used as default also on primitives that + * already have tags. + *

+ * Note: used for {@code addr:*} tags in {@code defaultpresets.xml}. + * + * @return true if see above + */ protected boolean isForceUseLastAsDefault() { return use_last_as_default == 2; } @@ -433,6 +400,11 @@ */ public void addListEntry(PresetListEntry e) { presetListEntries.add(e); + // we need to fix the entries because the XML Parser + // {@link org.openstreetmap.josm.tools.XmlObjectParser.Parser#startElement} has used the + // default standard constructor for {@link PresetListEntry} if the list entry was defined + // using XML {@code }. + e.cms = this; } /** @@ -445,10 +417,6 @@ } } - protected PresetListEntry getListEntry(String value) { - return presetListEntries.stream().filter(e -> Objects.equals(e.value, value)).findFirst().orElse(null); - } - @Override public MatchType getDefaultMatch() { return MatchType.NONE; Index: src/org/openstreetmap/josm/gui/tagging/presets/items/KeyedItem.java =================================================================== --- src/org/openstreetmap/josm/gui/tagging/presets/items/KeyedItem.java (revision 18252) +++ src/org/openstreetmap/josm/gui/tagging/presets/items/KeyedItem.java (working copy) @@ -7,9 +7,9 @@ import java.util.EnumSet; import java.util.HashMap; import java.util.Map; +import java.util.SortedMap; import java.util.NoSuchElementException; -import java.util.SortedSet; -import java.util.TreeSet; +import java.util.TreeMap; import javax.swing.JPopupMenu; @@ -26,9 +26,12 @@ */ public abstract class KeyedItem extends TextItem { - /** Translation of "<different>". Use in combo boxes to display an entry matching several different values. */ - protected static final String DIFFERENT = tr(""); + /** The constant value {@code ""}. */ + protected static final String DIFFERENT = ""; + /** Translation of {@code ""}. */ + protected static final String DIFFERENT_I18N = tr(DIFFERENT); + /** True if the default value should also be set on primitives that already have tags. */ protected static final BooleanProperty PROP_FILL_DEFAULT = new BooleanProperty("taggingpreset.fill-default-for-tagged-primitives", false); /** Last value of each key used in presets, used for prefilling corresponding fields */ @@ -99,10 +102,8 @@ * TODO merge with {@link org.openstreetmap.josm.data.osm.TagCollection} */ public static class Usage { - /** - * A set of values that were used for this key. - */ - public final SortedSet values = new TreeSet<>(); // NOSONAR + /** Usage count for all values used for this key */ + public final SortedMap map = new TreeMap<>(); private boolean hadKeys; private boolean hadEmpty; @@ -111,7 +112,7 @@ * @return true if there was exactly one value. */ public boolean hasUniqueValue() { - return values.size() == 1 && !hadEmpty; + return map.size() == 1 && !hadEmpty; } /** @@ -119,7 +120,7 @@ * @return true if it was unused. */ public boolean unused() { - return values.isEmpty(); + return map.isEmpty(); } /** @@ -128,7 +129,7 @@ * @throws NoSuchElementException if there is no such value. */ public String getFirst() { - return values.first(); + return map.firstKey(); } /** @@ -151,7 +152,7 @@ for (OsmPrimitive s : sel) { String v = s.get(key); if (v != null) { - returnValue.values.add(v); + returnValue.map.merge(v, 1, Integer::sum); } else { returnValue.hadEmpty = true; } @@ -167,7 +168,7 @@ for (OsmPrimitive s : sel) { String booleanValue = OsmUtils.getNamedOsmBoolean(s.get(key)); if (booleanValue != null) { - returnValue.values.add(booleanValue); + returnValue.map.merge(booleanValue, 1, Integer::sum); } } return returnValue; @@ -202,7 +203,7 @@ public Boolean matches(Map tags) { switch (MatchType.ofString(match)) { case NONE: - return null; + return null; // NOSONAR case KEY: return tags.containsKey(key) ? Boolean.TRUE : null; case KEY_REQUIRED: Index: src/org/openstreetmap/josm/gui/tagging/presets/items/MultiSelect.java =================================================================== --- src/org/openstreetmap/josm/gui/tagging/presets/items/MultiSelect.java (revision 18252) +++ src/org/openstreetmap/josm/gui/tagging/presets/items/MultiSelect.java (working copy) @@ -1,20 +1,17 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.gui.tagging.presets.items; -import java.awt.Dimension; -import java.util.Arrays; -import java.util.List; -import java.util.Set; -import java.util.TreeSet; +import java.util.TreeMap; +import java.util.stream.Collectors; -import javax.swing.JComponent; +import javax.swing.DefaultListModel; +import javax.swing.JLabel; import javax.swing.JList; import javax.swing.JPanel; import javax.swing.JScrollPane; -import javax.swing.ListModel; -import org.openstreetmap.josm.data.osm.Tag; import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetItemGuiSupport; +import org.openstreetmap.josm.gui.widgets.OrientationAction; import org.openstreetmap.josm.tools.GBC; /** @@ -27,88 +24,64 @@ */ public short rows; // NOSONAR - protected ConcatenatingJList list; + /** The model for the JList */ + protected final DefaultListModel model = new DefaultListModel<>(); + /** The swing component */ + protected final JList list = new JList<>(model); - @Override - protected JComponent addToPanelAnchor(JPanel p, String def, TaggingPresetItemGuiSupport support) { - list = new ConcatenatingJList(delimiter, presetListEntries.toArray(new PresetListEntry[0])); - ComboMultiSelectListCellRenderer renderer = new ComboMultiSelectListCellRenderer(list, list.getCellRenderer(), 200, key); - list.setCellRenderer(renderer); - Object itemToSelect = getItemToSelect(def, support, true); - list.setSelectedItem(itemToSelect == null ? null : new PresetListEntry(itemToSelect.toString())); - JScrollPane sp = new JScrollPane(list); - // if a number of rows has been specified in the preset, - // modify preferred height of scroll pane to match that row count. - if (rows > 0) { - double height = renderer.getListCellRendererComponent(list, - new PresetListEntry("x"), 0, false, false).getPreferredSize().getHeight() * rows; - sp.setPreferredSize(new Dimension((int) sp.getPreferredSize().getWidth(), (int) height)); + private void addEntry(PresetListEntry entry) { + if (!seenValues.containsKey(entry.value)) { + model.addElement(entry); + seenValues.put(entry.value, entry); } - list.addListSelectionListener(l -> support.fireItemValueModified(this, key, getSelectedValue())); - p.add(sp, GBC.eol().fill(GBC.HORIZONTAL)); - return list; } @Override - protected Object getSelectedItem() { - return list.getSelectedItem(); - } + protected boolean addToPanel(JPanel p, TaggingPresetItemGuiSupport support) { + initializeLocaleText(null); + usage = determineTextUsage(support.getSelected(), key); + seenValues = new TreeMap<>(); + initListEntries(); - @Override - public void addCommands(List changedTags) { - // Do not create any commands if list has been disabled because of an unknown value (fix #8605) - if (list.isEnabled()) { - super.addCommands(changedTags); + model.clear(); + if (!usage.hasUniqueValue() && !usage.unused()) { + addEntry(PresetListEntry.ENTRY_DIFFERENT); } - } - /** - * Class that allows list values to be assigned and retrieved as a comma-delimited - * string (extracted from TaggingPreset) - */ - private static class ConcatenatingJList extends JList { - private final char delimiter; - - protected ConcatenatingJList(char del, PresetListEntry... o) { - super(o); - delimiter = del; + String initialValue = getInitialValue(default_); + if (initialValue != null) { + // add all values already present to the list, otherwise we would remove all + // custom entries unknown to the preset + for (String value : initialValue.split(String.valueOf(delimiter), -1)) { + PresetListEntry e = new PresetListEntry(value, this); + addEntry(e); + int i = model.indexOf(e); + list.addSelectionInterval(i, i); + } } - public void setSelectedItem(Object o) { - if (o == null) { - clearSelection(); - } else { - String s = o.toString(); - Set parts = new TreeSet<>(Arrays.asList(s.split(String.valueOf(delimiter), -1))); - ListModel lm = getModel(); - int[] intParts = new int[lm.getSize()]; - int j = 0; - for (int i = 0; i < lm.getSize(); i++) { - final String value = lm.getElementAt(i).value; - if (parts.contains(value)) { - intParts[j++] = i; - parts.remove(value); - } - } - setSelectedIndices(Arrays.copyOf(intParts, j)); - // check if we have actually managed to represent the full - // value with our presets. if not, cop out; we will not offer - // a selection list that threatens to ruin the value. - setEnabled(parts.isEmpty()); - } + presetListEntries.forEach(this::addEntry); + + ComboMultiSelectListCellRenderer renderer = new ComboMultiSelectListCellRenderer(list, list.getCellRenderer(), 200, key); + list.setCellRenderer(renderer); + + if (rows > 0) { + list.setVisibleRowCount(rows); } + JLabel label = addLabel(p); + p.add(new JScrollPane(list), GBC.eol().fill(GBC.HORIZONTAL)); // NOSONAR + label.setLabelFor(list); - public String getSelectedItem() { - ListModel lm = getModel(); - int[] si = getSelectedIndices(); - StringBuilder builder = new StringBuilder(); - for (int i = 0; i < si.length; i++) { - if (i > 0) { - builder.append(delimiter); - } - builder.append(lm.getElementAt(si[i]).value); - } - return builder.toString(); - } + list.addListSelectionListener(l -> support.fireItemValueModified(this, key, getSelectedItem().value)); + list.setToolTipText(getKeyTooltipText()); + list.applyComponentOrientation(OrientationAction.getValueOrientation(key)); + + return true; } + + @Override + protected PresetListEntry getSelectedItem() { + return new PresetListEntry(list.getSelectedValuesList() + .stream().map(e -> e.value).collect(Collectors.joining(String.valueOf(delimiter))), this); + } } Index: src/org/openstreetmap/josm/gui/tagging/presets/items/PresetListEntry.java =================================================================== --- src/org/openstreetmap/josm/gui/tagging/presets/items/PresetListEntry.java (revision 18252) +++ src/org/openstreetmap/josm/gui/tagging/presets/items/PresetListEntry.java (working copy) @@ -16,15 +16,27 @@ /** * Preset list entry. + *

+ * Used for controls that offer a list of items to choose from like {@link Combo} and + * {@link MultiSelect}. */ public class PresetListEntry implements Comparable { - /** Entry value */ + /** Used to display an entry matching several different values. */ + protected static final PresetListEntry ENTRY_DIFFERENT = new PresetListEntry(KeyedItem.DIFFERENT, null); + /** Used to display an empty entry used to clear values. */ + protected static final PresetListEntry ENTRY_EMPTY = new PresetListEntry("", null); + + /** + * This is the value that is going to be written to the tag on the selected primitive(s). Except + * when the value is {@code ""}, which is never written, or the value is empty, which + * deletes the tag. {@code value} is never translated. + */ public String value; // NOSONAR - /** The context used for translating {@link #value} */ - public String value_context; // NOSONAR - /** Value displayed to the user */ + /** The ComboMultiSelect that displays the list */ + public ComboMultiSelect cms; // NOSONAR + /** Text displayed to the user instead of {@link #value}. */ public String display_value; // NOSONAR - /** Text to be displayed below {@code display_value}. */ + /** Text to be displayed below {@link #display_value} in the combobox list. */ public String short_description; // NOSONAR /** The location of icon file to display */ public String icon; // NOSONAR @@ -35,19 +47,28 @@ /** The localized version of {@link #short_description}. */ public String locale_short_description; // NOSONAR + private String cachedDisplayValue = null; + private String cachedShortDescription = null; + private ImageIcon cachedIcon = null; + /** * Constructs a new {@code PresetListEntry}, uninitialized. + * + * Public default constructor is needed by {@link org.openstreetmap.josm.tools.XmlObjectParser.Parser#startElement} */ public PresetListEntry() { - // Public default constructor is needed } /** - * Constructs a new {@code PresetListEntry}, initialized with a value. + * Constructs a new {@code PresetListEntry}, initialized with a value and + * {@link ComboMultiSelect} context. + * * @param value value + * @param cms the ComboMultiSelect */ - public PresetListEntry(String value) { + public PresetListEntry(String value, ComboMultiSelect cms) { this.value = value; + this.cms = cms; } /** @@ -60,14 +81,21 @@ * @return HTML formatted contents */ public String getListDisplay(int width) { - if (value.equals(KeyedItem.DIFFERENT)) { - return "" + KeyedItem.DIFFERENT + ""; + String displayValue = getDisplayValue(); + Integer count = cms == null ? null : cms.usage.map.get(value); + + if (count != null) { + displayValue = String.format("%s (%d)", displayValue, count); } - String shortDescription = getShortDescription(true); - String displayValue = getDisplayValue(); + if (this.equals(ENTRY_DIFFERENT)) { + return "" + Utils.escapeReservedCharactersHTML(displayValue) + ""; + } + String shortDescription = getShortDescription(); + if (shortDescription.isEmpty()) { + // avoids a collapsed list entry if value == "" if (displayValue.isEmpty()) { return " "; } @@ -86,36 +114,60 @@ * @return the entry icon, or {@code null} */ public ImageIcon getIcon() { - return icon == null ? null : TaggingPresetItem.loadImageIcon(icon, TaggingPresetReader.getZipIcons(), (int) icon_size); + if (icon != null && cachedIcon == null) { + cachedIcon = TaggingPresetItem.loadImageIcon(icon, TaggingPresetReader.getZipIcons(), (int) icon_size); + } + return cachedIcon; } /** - * Returns the contents of the current item view. + * Returns the contents displayed in the current item view. * @return the value to display */ public String getDisplayValue() { - return Utils.firstNonNull(locale_display_value, tr(display_value), trc(value_context, value)); + if (cachedDisplayValue == null) { + if (cms != null && cms.values_no_i18n) { + cachedDisplayValue = Utils.firstNonNull(value, " "); + } else { + cachedDisplayValue = Utils.firstNonNull( + locale_display_value, tr(display_value), trc(cms == null ? null : cms.values_context, value), " "); + } + } + return cachedDisplayValue; } /** * Returns the short description to display. - * @param translated whether the text must be translated * @return the short description to display */ - public String getShortDescription(boolean translated) { - String shortDesc = translated - ? Utils.firstNonNull(locale_short_description, tr(short_description)) - : short_description; - return shortDesc == null ? "" : shortDesc; + public String getShortDescription() { + if (cachedShortDescription == null) { + cachedShortDescription = Utils.firstNonNull(locale_short_description, tr(short_description), ""); + } + return cachedShortDescription; } + /** + * Returns the tooltip for this entry. + * @param key the tag key + * @return the tooltip + */ + public String getToolTipText(String key) { + if (this.equals(ENTRY_DIFFERENT)) { + return tr("Keeps the original values of the selected objects unchanged."); + } + if (value != null && !value.isEmpty()) { + return tr("Sets the key ''{0}'' to the value ''{1}''.", key, value); + } + return tr("Clears the key ''{0}''.", key); + } + // toString is mainly used to initialize the Editor @Override public String toString() { - if (KeyedItem.DIFFERENT.equals(value)) - return KeyedItem.DIFFERENT; - String displayValue = getDisplayValue(); - return displayValue != null ? displayValue.replaceAll("\\s*<.*>\\s*", " ") : ""; // remove additional markup, e.g.
+ if (this.equals(ENTRY_DIFFERENT)) + return getDisplayValue(); + return getDisplayValue().replaceAll("\\s*<.*>\\s*", " "); // remove additional markup, e.g.
} @Override @@ -133,6 +185,6 @@ @Override public int compareTo(PresetListEntry o) { - return AlphanumComparator.getInstance().compare(this.getDisplayValue(), o.getDisplayValue()); + return AlphanumComparator.getInstance().compare(this.value, o.value); } } Index: src/org/openstreetmap/josm/gui/tagging/presets/items/Text.java =================================================================== --- src/org/openstreetmap/josm/gui/tagging/presets/items/Text.java (revision 18252) +++ src/org/openstreetmap/josm/gui/tagging/presets/items/Text.java (working copy) @@ -137,12 +137,13 @@ // The selected primitives have different values for this key. Note: this // cannot be an AutoCompComboBox because the values in the dropdown are different from // those we autocomplete on. - JosmComboBox comboBox = new JosmComboBox<>(usage.values.toArray(new String[0])); + JosmComboBox comboBox = new JosmComboBox<>(); + comboBox.getModel().addAllElements(usage.map.keySet()); comboBox.setEditable(true); comboBox.setEditor(editor); - comboBox.getEditor().setItem(DIFFERENT); + comboBox.getEditor().setItem(DIFFERENT_I18N); value = comboBox; - originalValue = DIFFERENT; + originalValue = DIFFERENT_I18N; } initializeLocaleText(null); Index: test/unit/org/openstreetmap/josm/gui/tagging/presets/items/ComboTest.java =================================================================== --- test/unit/org/openstreetmap/josm/gui/tagging/presets/items/ComboTest.java (revision 18252) +++ test/unit/org/openstreetmap/josm/gui/tagging/presets/items/ComboTest.java (working copy) @@ -41,43 +41,70 @@ } /** - * Unit test for {@link ComboMultiSelect#use_last_as_default} and {@link ComboMultiSelect#getItemToSelect} + * Unit test for {@link ComboMultiSelect#use_last_as_default} and {@link ComboMultiSelect#getInitialValue} */ @Test void testUseLastAsDefault() { Combo combo = new Combo(); combo.key = "addr:country"; - combo.use_last_as_default = 1; combo.values_from = "java.util.Locale#getISOCountries"; OsmPrimitive way = OsmUtils.createPrimitive("way"); + OsmPrimitive wayTagged = OsmUtils.createPrimitive("way highway=residential"); OsmPrimitive wayAT = OsmUtils.createPrimitive("way addr:country=AT"); OsmPrimitive waySI = OsmUtils.createPrimitive("way addr:country=SI"); + KeyedItem.LAST_VALUES.clear(); + KeyedItem.LAST_VALUES.put("addr:country", "AT"); + combo.use_last_as_default = 0; combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, way)); - assertEquals("", combo.getSelectedValue()); + assertEquals("", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayTagged)); + assertEquals("", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT)); + assertEquals("AT", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, waySI)); + assertEquals("SI", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT, waySI)); + assertEquals(Combo.DIFFERENT, combo.getSelectedItem().value); - combo.default_ = "SI"; + combo.default_ = "AT"; combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, way)); - assertEquals("SI", combo.getSelectedValue()); + assertEquals("AT", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayTagged)); + assertEquals("", combo.getSelectedItem().value); combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT)); - assertEquals("AT", combo.getSelectedValue()); + assertEquals("AT", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, waySI)); + assertEquals("SI", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT, waySI)); + assertEquals(Combo.DIFFERENT, combo.getSelectedItem().value); combo.default_ = null; - KeyedItem.LAST_VALUES.clear(); - KeyedItem.LAST_VALUES.put("addr:country", "AT"); + combo.use_last_as_default = 1; // untagged objects only combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, way)); - assertEquals("AT", combo.getSelectedValue()); - combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(true, wayAT)); - assertEquals("AT", combo.getSelectedValue()); - combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(true)); - assertEquals("", combo.getSelectedValue()); - combo.use_last_as_default = 2; // "force" - combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(true)); - assertEquals("AT", combo.getSelectedValue()); + assertEquals("AT", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayTagged)); + assertEquals("", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT)); + assertEquals("AT", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, waySI)); + assertEquals("SI", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT, waySI)); + assertEquals(Combo.DIFFERENT, combo.getSelectedItem().value); + + combo.use_last_as_default = 2; // "force" on tagged objects too + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, way)); + assertEquals("AT", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayTagged)); + assertEquals("AT", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT)); + assertEquals("AT", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, waySI)); + assertEquals("SI", combo.getSelectedItem().value); + combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT, waySI)); + assertEquals(Combo.DIFFERENT, combo.getSelectedItem().value); + KeyedItem.LAST_VALUES.clear(); - - combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(true, wayAT, waySI)); - assertEquals(Combo.DIFFERENT, combo.getSelectedValue()); } @Test @@ -90,20 +117,20 @@ combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false)); assertEquals(5, combo.combobox.getItemCount()); combo.presetListEntries.stream().filter(e -> "red".equals(e.value)).findFirst().ifPresent(combo.combobox::setSelectedItem); - assertEquals("red", combo.getSelectedValue()); + assertEquals("red", combo.getSelectedItem().value); assertEquals("Rot", combo.getSelectedItem().toString()); assertEquals(new Color(0xFF0000), combo.getColor()); combo.presetListEntries.stream().filter(e -> "green".equals(e.value)).findFirst().ifPresent(combo.combobox::setSelectedItem); - assertEquals("green", combo.getSelectedValue()); + assertEquals("green", combo.getSelectedItem().value); assertEquals("Grün", combo.getSelectedItem().toString()); assertEquals(new Color(0x008000), combo.getColor()); combo.combobox.setSelectedItem("#135"); - assertEquals("#135", combo.getSelectedValue()); + assertEquals("#135", combo.getSelectedItem().value); assertEquals(new Color(0x113355), combo.getColor()); combo.combobox.setSelectedItem("#123456"); - assertEquals("#123456", combo.getSelectedValue()); + assertEquals("#123456", combo.getSelectedItem().value); assertEquals(new Color(0x123456), combo.getColor()); combo.setColor(new Color(0x448822)); - assertEquals("#448822", combo.getSelectedValue()); + assertEquals("#448822", combo.getSelectedItem().value); } } Index: test/unit/org/openstreetmap/josm/gui/tagging/presets/items/PresetListEntryTest.java =================================================================== --- test/unit/org/openstreetmap/josm/gui/tagging/presets/items/PresetListEntryTest.java (revision 18252) +++ test/unit/org/openstreetmap/josm/gui/tagging/presets/items/PresetListEntryTest.java (working copy) @@ -25,6 +25,6 @@ */ @Test void testTicket12416() { - assertTrue(new PresetListEntry("").getListDisplay(200).contains(" ")); + assertTrue(new PresetListEntry("", null).getListDisplay(200).contains(" ")); } }