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

Improve url form validation #117

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Improve url form validation #117

wants to merge 3 commits into from

Conversation

philmcmahon
Copy link
Contributor

What does this change?

It would be helpful to perform some client side validation on urls being added to the upload form so that we can, e.g. prevent people submitting invalid urls, or urls to youtube search results (as yt-dlp will then try download everything on the page).

This PR introduces this, by breaking up the existing 'textarea' into individual input elements, as suggested by @zekehuntergreen ages ago.

Before:
Screenshot 2025-01-10 at 16 43 30

After:

Screenshot 2025-01-10 at 16 44 24

How to test

You can run just the API and the client locally to test this - though hopefully screenshots make it fairly clear what's going on. I've not tested on CODE yet as I'm waiting for #115 to go out.

@philmcmahon philmcmahon requested a review from a team as a code owner January 10, 2025 16:45
@hoyla
Copy link
Contributor

hoyla commented Jan 10, 2025

In the "after" view it wasn't immediately clear to me which problem pertained to which URL, and I think it might be worse for red/green colourblind users. Maybe put a divider line between each URL field, so that any problem reporting appears adjacent to the URL it pertains to, with a line between that text and the following URL?

@philmcmahon
Copy link
Contributor Author

In the "after" view it wasn't immediately clear to me which problem pertained to which URL, and I think it might be worse for red/green colourblind users. Maybe put a divider line between each URL field, so that any problem reporting appears adjacent to the URL it pertains to, with a line between that text and the following URL?

How about this? Can make the line more obvious....
Screenshot 2025-01-10 at 17 16 59

@philmcmahon
Copy link
Contributor Author

Tiny bit wider!
Screenshot 2025-01-10 at 17 18 22

@hoyla
Copy link
Contributor

hoyla commented Jan 10, 2025

Oh, while you're tweaking this text can you use "URL" rather than "Url"? In both the widget label and on the Add Url button

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.

2 participants