Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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
Member

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
Contributor

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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.