From 083a88a4b2ecb00ea420075828cf33d86ab9c8f9 Mon Sep 17 00:00:00 2001 From: simonpoole Date: Tue, 26 Sep 2023 11:35:21 +0200 Subject: [PATCH] Support long text modal in additional items This will show the long text modal for "Additional" items if they are longer then the value set in the preferences. This set the default max length value in the preferences to 80 as otherwise the modal would essentially never be shown. Fixes https://github.com/MarcusWolschon/osmeditor4android/issues/2373 --- .../propertyeditor/PropertyEditorTest.java | 4 +- .../de/blau/android/prefs/Preferences.java | 2 +- .../propertyeditor/tagform/KeyValueRow.java | 9 +++ .../tagform/LongTextDialogRow.java | 3 +- .../tagform/TagFormFragment.java | 75 ++++++++++--------- src/main/java/de/blau/android/util/Util.java | 14 +++- src/main/res/xml-v19/advancedpreferences.xml | 2 +- src/main/res/xml-v24/advancedpreferences.xml | 2 +- src/main/res/xml-v29/advancedpreferences.xml | 2 +- src/main/res/xml/advancedpreferences.xml | 2 +- 10 files changed, 70 insertions(+), 45 deletions(-) diff --git a/src/androidTest/java/de/blau/android/propertyeditor/PropertyEditorTest.java b/src/androidTest/java/de/blau/android/propertyeditor/PropertyEditorTest.java index 63d1827e17..900dbb785c 100644 --- a/src/androidTest/java/de/blau/android/propertyeditor/PropertyEditorTest.java +++ b/src/androidTest/java/de/blau/android/propertyeditor/PropertyEditorTest.java @@ -485,7 +485,7 @@ public void longText() { found = TestUtils.clickText(device, true, getTranslatedPresetItemName(main, "Restaurant"), true, false); assertTrue(found); - // apply optional tags and check that diaper tag isn't present + // apply optional tags assertTrue(TestUtils.clickMenuButton(device, main.getString(R.string.tag_menu_apply_preset_with_optional), false, false)); TestUtils.scrollTo("Wheelchair access details", false); @@ -499,7 +499,7 @@ public void longText() { } catch (UiObjectNotFoundException e) { fail(); } - + assertTrue(TestUtils.findText(device, false, "1234567890"); TestUtils.clickHome(device, true); assertTrue(TestUtils.findText(device, false, context.getString(R.string.actionmode_nodeselect))); device.waitForIdle(); diff --git a/src/main/java/de/blau/android/prefs/Preferences.java b/src/main/java/de/blau/android/prefs/Preferences.java index d7c4949109..f97c369b7f 100755 --- a/src/main/java/de/blau/android/prefs/Preferences.java +++ b/src/main/java/de/blau/android/prefs/Preferences.java @@ -316,7 +316,7 @@ public Preferences(@NonNull Context ctx) { useImperialUnits = prefs.getBoolean(r.getString(R.string.config_useImperialUnits_key), false); - longStringLimit = getIntPref(R.string.config_longStringLimit_key, Capabilities.DEFAULT_MAX_STRING_LENGTH); + longStringLimit = getIntPref(R.string.config_longStringLimit_key, 80); } /** diff --git a/src/main/java/de/blau/android/propertyeditor/tagform/KeyValueRow.java b/src/main/java/de/blau/android/propertyeditor/tagform/KeyValueRow.java index 55ccf15cf7..c8c81ad770 100644 --- a/src/main/java/de/blau/android/propertyeditor/tagform/KeyValueRow.java +++ b/src/main/java/de/blau/android/propertyeditor/tagform/KeyValueRow.java @@ -1,5 +1,8 @@ package de.blau.android.propertyeditor.tagform; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + public interface KeyValueRow { /** @@ -7,6 +10,7 @@ public interface KeyValueRow { * * @return the key as a String */ + @NonNull String getKey(); /** @@ -14,5 +18,10 @@ public interface KeyValueRow { * * @return the current value as a String */ + @Nullable String getValue(); + + default boolean hasKey(@Nullable String key) { + return getKey().equals(key); + } } diff --git a/src/main/java/de/blau/android/propertyeditor/tagform/LongTextDialogRow.java b/src/main/java/de/blau/android/propertyeditor/tagform/LongTextDialogRow.java index 7f8d92cf35..845328c7c2 100644 --- a/src/main/java/de/blau/android/propertyeditor/tagform/LongTextDialogRow.java +++ b/src/main/java/de/blau/android/propertyeditor/tagform/LongTextDialogRow.java @@ -10,6 +10,7 @@ import android.widget.EditText; import android.widget.LinearLayout; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import androidx.appcompat.app.AlertDialog; import androidx.appcompat.app.AlertDialog.Builder; import de.blau.android.R; @@ -58,7 +59,7 @@ public LongTextDialogRow(@NonNull Context context, AttributeSet attrs) { * @return an instance of TagFormDialogRow */ static DialogRow getRow(@NonNull final TagFormFragment caller, @NonNull final LayoutInflater inflater, @NonNull final LinearLayout rowLayout, - @NonNull final PresetItem preset, @NonNull final PresetTextField field, @NonNull final String value, int maxLength) { + @Nullable final PresetItem preset, @NonNull final PresetTextField field, @NonNull final String value, int maxLength) { final DialogRow row = (DialogRow) inflater.inflate(R.layout.tag_form_text_dialog_row, rowLayout, false); String key = field.getKey(); String hint = field.getHint(); diff --git a/src/main/java/de/blau/android/propertyeditor/tagform/TagFormFragment.java b/src/main/java/de/blau/android/propertyeditor/tagform/TagFormFragment.java index e24716dd77..bfe71b3967 100644 --- a/src/main/java/de/blau/android/propertyeditor/tagform/TagFormFragment.java +++ b/src/main/java/de/blau/android/propertyeditor/tagform/TagFormFragment.java @@ -49,7 +49,6 @@ import de.blau.android.measure.Params; import de.blau.android.nsi.Names; import de.blau.android.osm.OsmElement; -import de.blau.android.osm.Server; import de.blau.android.osm.Tags; import de.blau.android.osm.Way; import de.blau.android.osm.Wiki; @@ -118,7 +117,8 @@ public class TagFormFragment extends BaseFragment implements FormUpdate { int maxInlineValues = 3; - int maxStringLength; // maximum key, value and role length + int maxStringLength; // maximum key, value and role length + private int longStringLimit; private Map displayOptional = new HashMap<>(); @@ -235,8 +235,8 @@ public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup c maxInlineValues = prefs.getMaxInlineValues(); - Server server = prefs.getServer(); - maxStringLength = server.getCachedCapabilities().getMaxStringLength(); + maxStringLength = propertyEditorListener.getCapabilities().getMaxStringLength(); + longStringLimit = prefs.getLongStringLimit(); if (displayMRUpresets) { de.blau.android.propertyeditor.Util.addMRUPresetsFragment(getChildFragmentManager(), R.id.mru_layout, @@ -303,7 +303,7 @@ ArrayAdapter getValueAutocompleteAdapter(@Nullable String key, @Nullable List @Nullable ArrayAdapter getValueAutocompleteAdapter(@Nullable String key, @Nullable List values, @Nullable PresetItem preset, @Nullable PresetTagField field, @NonNull Map allTags, boolean addRuler, boolean dedup, int addMruSize) { - if (key != null && key.length() > 0) { + if (Util.notEmpty(key)) { Set usedKeys = allTags.keySet(); if (TagEditorFragment.isStreetName(key, usedKeys)) { return nameAdapters.getStreetNameAdapter(values); @@ -397,7 +397,7 @@ ArrayAdapter getValueAutocompleteAdapter(@Nullable String key, @Nullable List } if (values != null) { // add in any non-standard non-empty values for (String value : values) { - if (value != null && !"".equals(value) && !counter.containsKey(value)) { + if (Util.notEmpty(value) && !counter.containsKey(value)) { StringWithDescription s = new StringWithDescription(value); adapter2.remove(s); adapter2.insert(s, 0); @@ -608,10 +608,10 @@ public void displayOptional(PresetItem presetItem, boolean optional) { /** * Return the view we have our rows in and work around some android craziness * - * @return the immutable row layout or null if it couldn't be found + * @return the non-preset row layout or null if it couldn't be found */ @Nullable - private View getImmutableView() { + private View getNonPresetView() { // android.support.v4.app.NoSaveStateFrameLayout View v = getView(); if (v != null) { @@ -657,7 +657,7 @@ public void update() { int pos = 0; ll.addView(editableView, pos++); - LinearLayout nonEditableView = (LinearLayout) getImmutableView(); + LinearLayout nonEditableView = (LinearLayout) getNonPresetView(); if (nonEditableView != null && nonEditableView.getChildCount() > 0) { nonEditableView.removeAllViews(); } @@ -819,7 +819,6 @@ private Map addTagsToViews(@NonNull EditableLayout editableView, linkedTags.put(field, value); keyToLinkedPreset.put(key, l); editableView.putTag(key, value); - i18nFound = true; tagList.remove(key); break; } @@ -948,6 +947,7 @@ void addRow(@Nullable final LinearLayout rowLayout, @NonNull final PresetTagFiel Log.e(DEBUG_TAG, "addRow called for fixed field " + field); return; } + final boolean longString = value != null && longStringLimit <= value.length(); if (preset == null) { // no preset here so we can only handle hardwired stuff specially if (key.endsWith(Tags.KEY_CONDITIONAL_SUFFIX)) { rowLayout.addView(getConditionalRestrictionDialogRow(rowLayout, null, null, key, value, null, allTags)); @@ -957,6 +957,10 @@ void addRow(@Nullable final LinearLayout rowLayout, @NonNull final PresetTagFiel rowLayout.addView(OpeningHoursDialogRow.getRow(this, inflater, rowLayout, null, null, key, value, null)); return; } + if (longString) { + rowLayout.addView(LongTextDialogRow.getRow(this, inflater, rowLayout, null, (PresetTextField) field, value, maxStringLength)); + return; + } rowLayout.addView(TextRow.getRow(this, inflater, rowLayout, null, new PresetTextField(key), value, null, allTags)); return; } @@ -971,7 +975,7 @@ void addRow(@Nullable final LinearLayout rowLayout, @NonNull final PresetTagFiel values = Util.wrapInList(value); } String hint = field.getHint(); - if (field.isDeprecated() && (hint != null && !"".equals(hint))) { + if (field.isDeprecated() && Util.notEmpty(hint)) { hint = getString(R.string.deprecated, hint); } // @@ -994,11 +998,8 @@ void addRow(@Nullable final LinearLayout rowLayout, @NonNull final PresetTagFiel rowLayout.addView(UrlDialogRow.getRow(this, inflater, rowLayout, preset, hint, key, value)); return; } - final int longStringLimit = prefs.getLongStringLimit(); - if (field instanceof PresetTextField - && (longStringLimit <= ((PresetTextField) field).length() || (value != null && longStringLimit <= value.length()))) { - rowLayout.addView(LongTextDialogRow.getRow(this, inflater, rowLayout, preset, (PresetTextField) field, value, - propertyEditorListener.getCapabilities().getMaxStringLength())); + if (field instanceof PresetTextField && (longStringLimit <= ((PresetTextField) field).length() || longString)) { + rowLayout.addView(LongTextDialogRow.getRow(this, inflater, rowLayout, preset, (PresetTextField) field, value, maxStringLength)); return; } rowLayout.addView(TextRow.getRow(this, inflater, rowLayout, preset, field, value, values, allTags)); @@ -1047,7 +1048,7 @@ void addRow(@Nullable final LinearLayout rowLayout, @NonNull final PresetTagFiel StringWithDescription tempOff = ((PresetCheckField) field).getOffValue(); final String valueOff = tempOff == null ? "" : tempOff.getValue(); String description = tempOff == null ? "" : tempOff.getDescription(); - if (description == null) { + if (Util.isEmpty(description)) { description = valueOff; } final CheckRow row = (CheckRow) inflater.inflate(R.layout.tag_form_check_row, rowLayout, false); @@ -1093,10 +1094,11 @@ public boolean isOpeningHours(@NonNull final String key, @NonNull ValueType valu * @param value existing value for the tag * @param values a list containing all the predefined values in the PresetItem for the key * @param allTags a Map of the tags currently being edited - * @return a TagFormDialogRow instance + * @return a DialogRow instance */ - private DialogRow getConditionalRestrictionDialogRow(LinearLayout rowLayout, PresetItem preset, final String hint, final String key, final String value, - @Nullable final List values, Map allTags) { + @NonNull + private DialogRow getConditionalRestrictionDialogRow(@Nullable LinearLayout rowLayout, @Nullable PresetItem preset, @Nullable final String hint, + @Nullable final String key, @Nullable final String value, @Nullable final List values, Map allTags) { final DialogRow row = (DialogRow) inflater.inflate(R.layout.tag_form_combo_dialog_row, rowLayout, false); row.keyView.setText(hint != null ? hint : key); row.keyView.setTag(key); @@ -1112,7 +1114,7 @@ private DialogRow getConditionalRestrictionDialogRow(LinearLayout rowLayout, Pre StringWithDescription swd = new StringWithDescription(o); Log.d(DEBUG_TAG, "adding " + swd); String v = swd.getValue(); - if (v == null || "".equals(v)) { + if (Util.isEmpty(v)) { continue; } Log.d(DEBUG_TAG, "adding " + v + " to templates"); @@ -1120,8 +1122,11 @@ private DialogRow getConditionalRestrictionDialogRow(LinearLayout rowLayout, Pre } } } - if (value != null && !"".equals(value)) { - ConditionalRestrictionParser parser = new ConditionalRestrictionParser(new ByteArrayInputStream(value.getBytes())); + if (Util.notEmpty(value)) { + ConditionalRestrictionParser parser = new ConditionalRestrictionParser(new ByteArrayInputStream(value.getBytes())); // NOSONAR + // can't + // be + // null try { row.setValue(ch.poole.conditionalrestrictionparser.Util.prettyPrint(parser.restrictions())); } catch (Exception ex) { @@ -1165,7 +1170,7 @@ public boolean focusOnTag(@NonNull String key) { for (int i = ll2.getChildCount() - 1; i >= 0; --i) { View v = ll2.getChildAt(i); boolean isTextRow = v instanceof TextRow; - if ((v instanceof DialogRow || isTextRow) && ((KeyValueRow) v).getKey().equals(key)) { + if ((v instanceof DialogRow || isTextRow) && ((KeyValueRow) v).hasKey(key)) { Util.scrollToRow(sv, v, true, true); if (isTextRow) { ((TextRow) v).getValueView().requestFocus(); @@ -1190,7 +1195,7 @@ private boolean focusOnEmpty() { boolean found = false; LinearLayout ll = (LinearLayout) getView().findViewById(R.id.form_container_layout); if (ll == null) { - Log.d(DEBUG_TAG, "update container layout null"); + Log.d(DEBUG_TAG, "container layout null"); return false; } int pos = 0; @@ -1218,22 +1223,22 @@ private boolean focusOnEmpty() { */ @Nullable public View getRow(@NonNull String key) { - View sv = getView(); - LinearLayout ll = (LinearLayout) sv.findViewById(R.id.form_container_layout); + LinearLayout ll = (LinearLayout) getView().findViewById(R.id.form_container_layout); if (ll != null) { int pos = 0; while (pos < ll.getChildCount()) { final View child = ll.getChildAt(pos); - if (child instanceof LinearLayout) { - LinearLayout ll2 = (LinearLayout) child; - for (int i = ll2.getChildCount() - 1; i >= 0; --i) { - View v = ll2.getChildAt(i); - if ((v instanceof TextRow || v instanceof DialogRow) && ((KeyValueRow) v).getKey().equals(key)) { - return v; - } + pos++; + if (!(child instanceof LinearLayout)) { + continue; + } + LinearLayout ll2 = (LinearLayout) child; + for (int i = ll2.getChildCount() - 1; i >= 0; --i) { + View v = ll2.getChildAt(i); + if (v instanceof KeyValueRow && ((KeyValueRow) v).hasKey(key)) { + return v; } } - pos++; } } return null; diff --git a/src/main/java/de/blau/android/util/Util.java b/src/main/java/de/blau/android/util/Util.java index b525499a64..a2f1b5a144 100644 --- a/src/main/java/de/blau/android/util/Util.java +++ b/src/main/java/de/blau/android/util/Util.java @@ -560,7 +560,7 @@ public static boolean supportsWebView(@NonNull Context ctx) { * @param packageManager a PackageManager instance * @return true if the package is installed */ - + public static boolean isPackageInstalled(@NonNull String packageName, @NonNull PackageManager packageManager) { try { getPackageInfo(packageName, packageManager); @@ -680,7 +680,17 @@ public static String elementTypeId(@NonNull Context ctx, @NonNull String type, l * @return true if text is neither null nor the empty String */ public static boolean notEmpty(@Nullable final String text) { - return text != null && !"".equals(text); + return text != null && text.length() > 0; + } + + /** + * Check if a String is either null or the empty String + * + * @param text the input String + * @return true if text is either null or the empty String + */ + public static boolean isEmpty(@Nullable final String text) { + return text == null || text.length() == 0; } /** diff --git a/src/main/res/xml-v19/advancedpreferences.xml b/src/main/res/xml-v19/advancedpreferences.xml index 57923883b3..8ee5174884 100644 --- a/src/main/res/xml-v19/advancedpreferences.xml +++ b/src/main/res/xml-v19/advancedpreferences.xml @@ -113,7 +113,7 @@ app:spt_currentValueText="@string/config_maxInlineValues_current" app:spt_setWrapSelectorWheel="false" />