Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce printSetInputMaxlength to properly set an inputs maxlength … #4296

Merged
merged 3 commits into from
Nov 21, 2024

Conversation

WouterGritter
Copy link
Contributor

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
Copy link
Contributor Author

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

@softhack007
Copy link
Collaborator

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

Hi,

sorry HTML/JS is not my area.

@blazoncek
Copy link
Collaborator

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
Copy link
Contributor Author

WouterGritter commented Nov 21, 2024

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
Copy link
Contributor Author

I think we can merge this now.

@netmindz netmindz merged commit 89d587e into wled:0_15 Nov 21, 2024
@WouterGritter WouterGritter deleted the mqtt-fix-settings-input-maxlength branch November 22, 2024 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants