Set a max length for many inputs

[kommunarr]

Set a max length for many inputs

Pull Request Type

• Feature Implementation

Related issue

#5150 (comment)

Description

There's just no need for that much information in a custom label, because beyond a certain point, we're already limiting the visible text.

Excluded are places where an unconstrained input is valid:

• Top Nav search bar
• SponsorBlock Settings
• Proxy Settings
• External Player Settings
• Player Settings
• Password Settings
• General Settings -> Invidious Instance

Testing

• Test that appropriate inputs are limited from continued text
• (Optional) Test edge case behavior when a playlist title is already above the max # characters and the title is edited. Expected outcome: you can't add new characters, but you can remove them, and save at any length.

Desktop

• OS: OpenSUSE
• OS Version: TW

Additional context

The User Playlist desktop list view breaks for long enough titles and descriptions. We probably need code truncating those after x lines. The descriptions are shown in full on mobile view, so it's still useful for them to be able to be longer than that.

[absidue]

Please make the max length opt-in, that means the default value is null and then you pass in the limit in the few places it is needed, making it opt-out is going to cause more problems than it will solve. For example you currently set a 100 character limit on the search bar, with the reasoning that it makes the search query limit code obsolete, however that will break the second use of the search bar: handling URLs. The URLs can easily go over 100 characters, especially if they come from a tor Invidious instance, see this comment for more information: #4288 (comment)

[kommunarr]

Thanks for this context @absidue, I wasn't aware of that. Will update.

[kommunarr]

Updated now to remove the constraints from places where it would restrict valid inputs. Kept the default value the same so we don't have to have a magic number, or a constant we'd have to import into each file.

[PikachuEXE]

Please make the max length opt-in, that means the default value is null and then you pass in the limit in the few places it is needed, making it opt-out is going to cause more problems than it will solve.

I don't think this is solved
And also now I have no idea what to test (coz the diff doesn't show the list of inputs

[kommunarr]

I don't think this is solved

See answer:

Kept the default value the same so we don't have to have a magic number, or a constant we'd have to import into each file.

And also now I have no idea what to test (coz the diff doesn't show the list of inputs

Thanks, just updated the OP with the list of non-affected inputs.

[PikachuEXE]

• Playlist Name should be 255 at least (or unlimited)
• Playlist Description should be 255 at least (or unlimited)
• Proxy Port Number should be 5

Also in #5150 (comment) I simply means limiting the displayed value not input value

[absidue]

I still think it would be better to get rid of the implicit limit of 100 and instead set explicit limits where they are actually required. Your argument was that we would need a magic constant imported into multiple files, but I would argue that we don't need a constant at all, just write the number where it is needed. Secondly the current situation is worse, because it's still a magic number, but you don't see it at the call/usage site, so you won't even know it exists unless you stumble upon it in testing or remember to look at the ft-input component. It also means we won't have to override it with null in so many places anymore.

[absidue]

Other than that it looks good now, thank you for doing the changes.