From 2c8d7d6ae2e6e935d2d7963a1fd7b5900c7f8e57 Mon Sep 17 00:00:00 2001 From: Amolith Date: Wed, 1 Jul 2026 13:50:14 -0600 Subject: [PATCH] Bind sliders from a parsed spec Keeping field classification and binding in the same contract instead of re-parsing the form field in two places --- .../conversations/entities/Conversation.java | 39 ++++----- .../entities/ConversationTest.java | 80 +++++++++++-------- 2 files changed, 60 insertions(+), 59 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/entities/Conversation.java b/src/main/java/eu/siacs/conversations/entities/Conversation.java index e5347fd244a16b39cae96c19b86369df8bf92212..ad89faf19a749d1f89c8c4e41e699c2962d6c503 100644 --- a/src/main/java/eu/siacs/conversations/entities/Conversation.java +++ b/src/main/java/eu/siacs/conversations/entities/Conversation.java @@ -1869,7 +1869,9 @@ public class Conversation extends AbstractEntity } } - static Float steppedSliderStep(final Element field) { + record SliderSpec(String datatype, float min, float max, float value, float step) {} + + static SliderSpec sliderSpec(final Element field) { final Element validate = field.findChild("validate", "http://jabber.org/protocol/xdata-validate"); final String datatype = validate == null ? null : validate.getAttribute("datatype"); if (!isNumericDatatype(datatype)) return null; @@ -1904,7 +1906,9 @@ public class Conversation extends AbstractEntity } } if (step == null || (step > 0f && !landsOnStep(max, min, step))) return null; - return step == 0f || landsOnStep(parsedValue, min, step) ? step : null; + return step == 0f || landsOnStep(parsedValue, min, step) + ? new SliderSpec(datatype, min, max, parsedValue, step) + : null; } static String formatSliderValue(final float value, final String datatype) { @@ -1941,10 +1945,6 @@ public class Conversation extends AbstractEntity return values; } - private static Float rangeFloat(final Element range, final String name) { - return range == null ? null : parseFloat(range.getAttribute(name)); - } - private static Float parseFloat(final String value) { if (value == null || value.equals("")) return null; try { @@ -3059,34 +3059,23 @@ public class Conversation extends AbstractEntity @Override public void bind(Item item) { final Field boundField = (Field) item; - final Element validate = boundField.el.findChild("validate", "http://jabber.org/protocol/xdata-validate"); - final String datatype = validate == null ? null : validate.getAttribute("datatype"); - final Element range = validate == null ? null : validate.findChild("range", "http://jabber.org/protocol/xdata-validate"); - final Float step = steppedSliderStep(boundField.el); - final Float min = rangeFloat(range, "min"); - final Float max = rangeFloat(range, "max"); - if (step == null || min == null || max == null) { + final SliderSpec spec = sliderSpec(boundField.el); + if (spec == null) { throw new IllegalStateException("Invalid slider field bound to slider view holder"); } - float value = min; - final Float parsedValue = parseFloat(firstValue(boundField.el)); - if (parsedValue != null && parsedValue >= min && parsedValue <= max) { - value = parsedValue; - } - field = boundField; binding.slider.clearOnChangeListeners(); setTextOrHide(binding.label, field.getLabel()); setTextOrHide(binding.desc, field.getDesc()); - binding.slider.setValueFrom(min); - binding.slider.setValueTo(max); - binding.slider.setValue(value); - binding.slider.setStepSize(step); + binding.slider.setValueFrom(spec.min()); + binding.slider.setValueTo(spec.max()); + binding.slider.setValue(spec.value()); + binding.slider.setStepSize(spec.step()); binding.slider.addOnChangeListener((slider, sliderValue, fromUser) -> { if (!fromUser) return; - field.setValues(List.of(formatSliderValue(sliderValue, datatype))); + field.setValues(List.of(formatSliderValue(sliderValue, spec.datatype()))); }); } } @@ -3278,7 +3267,7 @@ public class Conversation extends AbstractEntity range != null && range.getAttribute("min") != null && range.getAttribute("max") != null && isNumericDatatype(datatype) ) { // has a range and is numeric, use a slider if it can represent every valid value - viewType = steppedSliderStep(el) == null ? TYPE_TEXT_FIELD : TYPE_SLIDER_FIELD; + viewType = sliderSpec(el) == null ? TYPE_TEXT_FIELD : TYPE_SLIDER_FIELD; } else if (fieldType.equals("list-single")) { if (fillableFieldCount == 1 && actionsAdapter.countProceed() < 1 && Option.forField(el).size() < 50) { viewType = TYPE_BUTTON_GRID_FIELD; diff --git a/src/test/java/eu/siacs/conversations/entities/ConversationTest.java b/src/test/java/eu/siacs/conversations/entities/ConversationTest.java index bc380469f5c77cf8ad0d11307a9adf88f58118c4..748a20a1f4bbf863d1cc74bf2f29ff8ac832961b 100644 --- a/src/test/java/eu/siacs/conversations/entities/ConversationTest.java +++ b/src/test/java/eu/siacs/conversations/entities/ConversationTest.java @@ -182,84 +182,82 @@ public class ConversationTest { } @Test - public void steppedSliderStepRejectsInRangeValueMissingFromOptionLattice() { + public void sliderSpecRejectsInRangeValueMissingFromOptionLattice() { final var field = sliderField("xs:integer", "0", "68", "7", "0", "34", "68"); - Assert.assertNull( - "A value the option-derived slider cannot represent should fall back to text input", - Conversation.steppedSliderStep(field)); + assertNotSlider("A value the option-derived slider cannot represent should fall back to text input", field); } @Test - public void steppedSliderStepAcceptsCompatibleOptionLattice() { + public void sliderSpecAcceptsCompatibleOptionLattice() { final var field = sliderField("xs:integer", "0", "70", "35", "0", "35", "70"); - Assert.assertEquals(Float.valueOf(35f), Conversation.steppedSliderStep(field)); + assertSliderStep(field, 35f); } @Test - public void steppedSliderStepUsesIntegerStepWithoutOptions() { + public void sliderSpecUsesIntegerStepWithoutOptions() { final var field = sliderField("xs:integer", "0", "10", "7"); - Assert.assertEquals(Float.valueOf(1f), Conversation.steppedSliderStep(field)); + assertSliderStep(field, 1f); } @Test - public void steppedSliderStepUsesContinuousStepWithoutOptionsForDecimal() { + public void sliderSpecUsesContinuousStepWithoutOptionsForDecimal() { final var field = sliderField("xs:decimal", "0", "1", "0.000001"); - Assert.assertEquals(Float.valueOf(0f), Conversation.steppedSliderStep(field)); + assertSliderStep(field, 0f); } @Test - public void steppedSliderStepRejectsMissingOrMalformedRangeBounds() { + public void sliderSpecRejectsMissingOrMalformedRangeBounds() { final var missingMin = sliderField("xs:integer", null, "10", "5"); final var malformedMax = sliderField("xs:integer", "0", "not-a-number", "5"); - Assert.assertNull(Conversation.steppedSliderStep(missingMin)); - Assert.assertNull(Conversation.steppedSliderStep(malformedMax)); + assertNotSlider(missingMin); + assertNotSlider(malformedMax); } @Test - public void steppedSliderStepRejectsIntegerValueThatCannotLandOnStep() { + public void sliderSpecRejectsIntegerValueThatCannotLandOnStep() { final var field = sliderField("xs:integer", "0", "10", "0.5"); - Assert.assertNull(Conversation.steppedSliderStep(field)); + assertNotSlider(field); } @Test - public void steppedSliderStepRejectsFractionalIntegerBounds() { + public void sliderSpecRejectsFractionalIntegerBounds() { final var field = sliderField("xs:integer", "0.5", "10.5", "1.5"); - Assert.assertNull(Conversation.steppedSliderStep(field)); + assertNotSlider(field); } @Test - public void steppedSliderStepRejectsFractionalIntegerOptions() { + public void sliderSpecRejectsFractionalIntegerOptions() { final var field = sliderField("xs:integer", "0", "1", "0", "0", "0.5", "1"); - Assert.assertNull(Conversation.steppedSliderStep(field)); + assertNotSlider(field); } @Test - public void steppedSliderStepRejectsFractionalIntegerValueAtFloatPrecisionBoundary() { + public void sliderSpecRejectsFractionalIntegerValueAtFloatPrecisionBoundary() { final var field = sliderField("xs:integer", "16777216", "16777218", "16777216.5"); - Assert.assertNull(Conversation.steppedSliderStep(field)); + assertNotSlider(field); } @Test - public void steppedSliderStepRejectsIntegerValueRoundedByFloatParsing() { + public void sliderSpecRejectsIntegerValueRoundedByFloatParsing() { final var field = sliderField("xs:integer", "16777216", "16777218", "16777217"); - Assert.assertNull(Conversation.steppedSliderStep(field)); + assertNotSlider(field); } @Test - public void steppedSliderStepRejectsIntegerRangeWithUnrepresentableIntermediateValue() { + public void sliderSpecRejectsIntegerRangeWithUnrepresentableIntermediateValue() { final var field = sliderField("xs:integer", "16777216", "16777218", "16777216"); - Assert.assertNull(Conversation.steppedSliderStep(field)); + assertNotSlider(field); } @Test @@ -273,33 +271,33 @@ public class ConversationTest { } @Test - public void steppedSliderStepRejectsIncompatibleBounds() { + public void sliderSpecRejectsIncompatibleBounds() { final var field = sliderField("xs:integer", "0", "70", "34", "0", "34", "68"); - Assert.assertNull(Conversation.steppedSliderStep(field)); + assertNotSlider(field); } @Test - public void steppedSliderStepRejectsMalformedOptions() { + public void sliderSpecRejectsMalformedOptions() { final var field = sliderField("xs:integer", "0", "70", "35", "0", "not-a-number", "70"); - Assert.assertNull(Conversation.steppedSliderStep(field)); + assertNotSlider(field); } @Test - public void steppedSliderStepRejectsValuesOutsideRange() { + public void sliderSpecRejectsValuesOutsideRange() { final var below = sliderField("xs:integer", "0", "70", "-5", "0", "35", "70"); final var above = sliderField("xs:integer", "0", "70", "999", "0", "35", "70"); - Assert.assertNull(Conversation.steppedSliderStep(below)); - Assert.assertNull(Conversation.steppedSliderStep(above)); + assertNotSlider(below); + assertNotSlider(above); } @Test - public void steppedSliderStepRejectsDuplicateOptions() { + public void sliderSpecRejectsDuplicateOptions() { final var field = sliderField("xs:integer", "0", "70", "35", "0", "35", "35", "70"); - Assert.assertNull(Conversation.steppedSliderStep(field)); + assertNotSlider(field); } @Test @@ -433,6 +431,20 @@ public class ConversationTest { } } + private static void assertSliderStep(final Element field, final float step) { + final var spec = Conversation.sliderSpec(field); + Assert.assertNotNull("Field should be usable as a slider", spec); + Assert.assertEquals(step, spec.step(), 0.0001f); + } + + private static void assertNotSlider(final Element field) { + Assert.assertNull(Conversation.sliderSpec(field)); + } + + private static void assertNotSlider(final String message, final Element field) { + Assert.assertNull(message, Conversation.sliderSpec(field)); + } + private static Element sliderField( final String datatype, final String min,