WiFiManagerParameter Encapsulated HTML Generation #1774
Conversation
This does *not* refer to the length of the string passed in, or the length of the string currently stored. This refers to the maximum length of the HTML form, and the size of the class buffer to hold the value. Note that this does *not* change the internal _length name, because the variable is exposed as protected and through friendship with WiFiManager. It is however marked deprecated and can be changed on the next major release.
To clarify that the function returns the *max* possible length, and not the length of the current value. Deprecating getValueLength for removal on next major release.
In HTML forms names and IDs are two different things. Although this value is applied to both attributes, the form data corresponds to the name, *not* the ID. This causes confusion if we want the name and ID to differ, since we are currently calling the name the ID.
For intercepting the value received from the server in derived classes
Preventing a new default value from being assigned if the max length is set to a negative value
Because this is polymorphic, the destructor needs to be virtual to allow for properly clearing data
This creates better encapsulation, and allows it to be overridden by child classes.
This is unused and awkward because it must be done within the manager class to use the loop parameter, since passing the loop value to the parameter object would be odd. Removing it makes the most sense, although it is technically a breaking change if the user has custom string files.
|
I think I started adding a param index, so parameters didn't need an explicit id/name in the case of dynamic or stateful params etc |
|
Also why did you change id() to name(), wont this break anyones implementation of a param child class ? |
That makes sense. Although with this change to move HTML generation to the parameter class, I'd suggest that the decision for how to handle a lack of a name/id should be the responsibility of whatever derived class chooses to omit it.
No, the change from 'id' to 'name' should not break API compatibility. The API changes were submitted separately in #1773. Those commits were included here because this also modifies the parameter class, and not including them would otherwise create a conflict when (if) both PRs are merged. |
|
Yeah I recall now, my bad, finally reviewing this |
This PR encapsulates the
WiFiManagerParameterHTML generation as part of the class, rather than as a function withinWiFiManager. This allows derived classes to define their own HTML output and greatly increases encapsulation:This also greatly simplifies the
getParamOut()function ofWiFiManager:... // add the extra parameters to the form for (int i = 0; i < _paramsCount; i++) { page += _params[i]->getHTML(); } ...This is a non-breaking change and should not modify the output HTML, with the exception of
{I}as noted below.Note that this PR contains a potentially breaking change, by removing token I (
T_I/{I}/ capital 'i') processing for parameters which include that token in their string. This was removed because the parameter HTML is no longer generated in the context of the loop, and I didn't see a compelling reason to pass the iterator to the class function.From what I can see parsing through the history of the
strings_enfile that feature was never used by the library itself, but it could potentially affect a user somewhere with custom string files (HTTP_FORM_LABEL/HTTP_FORM_PARAM). If that's objectionable I'm happy to revert that commit (c4665fc).Please note that this PR incorporates the necessary API changes from #1773.