Remove maxlength attribute from textarea after character count JavaScript has been initialised

[andymantell]

If a maxlength attribute is specified on the textarea, this should be removed after the character-count JS has kicked in. At the moment however, the JS is trying to remove the attribute from the wrapping div which does not do anything.

[andymantell]

Only saw the big refactor going on over in #2577 after I'd opened this. Might just be something to fold into that? Up to you anyway /shrug

[36degrees]

Great spot @andymantell – thanks!

Digging in to the history, it looks like this might have been broken since the component was introduced in #959 as we've always tried to remove the maxlength from the $module.

I think originally we had planned to set the character limit using the maxlength attribute. We to avoid maxlength elsewhere because if users hit the limit they are unable to enter any more characters but don't get any feedback to tell them why. So we opted to use a data attribute instead, which has no effect when the JS doesn't kick in, and rely on server-side validation to enforce the limit.

There was some discussion about the value of removing the maxlength attribute in that PR, presumably as the code was refactored when that decision was made?

I definitely agree that the current code is 'broken' as it doesn't do anything useful, but I am wondering if we should just remove it. As far as I know, the only way to set maxlength using the Nunjucks macro is to set it as part of the attributes object, and we would generally prefer users didn't set maxlength at all.

@andymantell Are you setting maxlength and therefore reliant on this behaviour and its fix? Or did you just spot the mistake?

[andymantell]

I've got maxlength set at the moment, because I'm starting building something on 111 online from a no-JS perspective and half figured it was best to have it in there. But I am very open to being convinced that I shouldn't use it at all. Of course I'll still need some serverside validation anyway.

So yeah I'm not reliant on it at all - if that's the recommendation, I'm quite happy to drop it from what I'm building and we can close this off.

[andymantell]

Although I guess the purpose of this line of code is still valid - that if someone like myself has added maxlength in for whatever reason (even if that reason is questionable), you really don't want it left in there as it scuppers the character count code.

Therefore, you should maybe keep it in? And forcibly remove it from the textarea.

[vanitabarrett]

Thanks for raising this PR @andymantell . A couple of us have had a discussion about this and while I think we're a bit 50/50 on whether to accept the PR and fix the logic or just remove it entirely, the easiest option for us right now would be to fix what this line was originally intended to do.

@andymantell are you ok to rebase this PR against the latest changes in main before I give it a review?

[andymantell]

Am away at the moment but I will do this when I'm back on Tuesday if that's ok. Thanks.

[andymantell]

@vanitabarrett This is up to date with main now and conflicts resolved 👍🏻

[vanitabarrett]

Looks good to me. This just needs to be approved by someone else in the team, and then it can be merged.

[andymantell]

Thanks all 👍🏻