Aircoookie / WLED

Control WS2812B and many more types of digital RGB LEDs with an ESP8266 or ESP32 over WiFi!
https://kno.wled.ge
MIT License
15.04k stars 3.25k forks source link

Introduce printSetInputMaxlength to properly set an inputs maxlength … #4296

Closed WouterGritter closed 18 hours ago

WouterGritter commented 1 day ago

Relates to !4295 by fixing MQTT related code that deals with the MQTT_MAX_TOPIC_LEN variable.

Previously, the maxlength attribute on the <input> fields in settings_sync.html were set with inputElement.maxlength = <length>, however this did not seem to actually modify the maxlength attribute as seen in the screenshot (also I couldn't type any more characters than the previous maxlength setting): image

As seen, modifying element.maxlength does actually modify the variable, but it does not modify the maxlength attribute weirdly. Using element.setAttribute("maxlength", X) was the fix.

In this PR I've modified the code to implement a function to use this JS line instead of the one previously used.

WouterGritter commented 1 day ago

Tagging you @softhack007 as you're aware of the context of the previous PR.

softhack007 commented 1 day ago

Tagging you @softhack007 as you're aware of the context of the previous PR.

Hi,

sorry HTML/JS is not my area.

blazoncek commented 1 day ago

If I was doing something like general method for modifying any attribute, I would have done it something like this:

function upAttr(o,a,v)
{
    d.Sf[o].setAttribute(a,v);
}
settingsScript.printf_P(PSTR("upAttr('MD','maxlength',%d);upAttr('MG','maxlength',%d);upAttr('MS','maxlength',%d);"),
   MQTT_MAX_TOPIC_LEN, MQTT_MAX_TOPIC_LEN, MQTT_MAX_SERVER_LEN);

But particular case is only the issue of missing uppercase letter. Instead of maxlength JavaScript access needs property called maxLength.

So, according to W3C Schools, the solution with least change is:

settingsScript.printf_P(PSTR("d.Sf.MD.maxLength=%d;d.Sf.MG.maxLength=%d;d.Sf.MS.maxLength=%d;"),
              MQTT_MAX_TOPIC_LEN, MQTT_MAX_TOPIC_LEN, MQTT_MAX_SERVER_LEN);
WouterGritter commented 1 day ago

Damn, great catch. I agree with making the least amount of changes required. Will replace maxlength with maxLength like you proposed (tested it and it works).

WouterGritter commented 1 day ago

I think we can merge this now.