omeryusufyagci
commented
1 month ago Hi @Soyvor, thanks for your contributions and for taking the time to address the issues you found. I appreciate your effort and recognize that you've identified valid points for improvement, particularly regarding the frontend, which indeed hasn't received much attention so far.
Feedback on (#15):
The current error handling is minimal, only showing "An error occurred!". It could be enhanced to give more specific feedback to the user (e.g., network issues, incorrect URL format).
You mentioned the error handling is minimal, and I agree it could be improved. The change you made to indicate "during processing" is a good start, but it's somewhat limited. Did you intend to expand this further?
The url field does not validate if the input is a valid YouTube URL. You should add basic validation to ensure the URL is well-formed before sending it to the server.
I understand the need for validating URLs. Currently, we're using YouTube because it speaks to a wide audience, but as we prepare for live stream support, we plan to accept URLs from other domains as well. If you want to add validation, it would be great to consider a more generic validator that's extensible, rather than focusing solely on YouTube URLs.
Video display and button disable are good additions
The backend already includes basic sanitization, which can definitely be extended. However, I would prefer to focus on this in a dedicated issue.
Feedback on this issue (#16): Spinner will be nice, sure!
YT video validation was covered above.
Direct file upload would be very good! As well as providing a download link for the processed video. These together would make a nice addition!
Optionally add an audio player alongside the video player to allow users to listen to the audio without music (assuming this is the app’s purpose).
The main purpose of the app is to efficiently filter videos, and while we do generate intermediary products like isolated vocals, adding an audio player doesn’t align with the main goal at this stage.
Display a success message with more details about the processed video (such as title, duration) after processing is completed.
This, along with the screen reader, and dynamic responsiveness were not addressed in your PR. Did you intend to add them later on?
Summary:
You've highlighted some important points, and I appreciate your contributions. To proceed, I will close the two issues discussed here and ask that you open a new Improvement Request with a well-defined scope, covering the changes you'd like to make.
Once you update your PR (#17) to align with this new consolidated issue, I'll be happy to review it and apply the hacktoberfest label if it meets the criteria.
A couple of quick notes:
- Please try to adhere to the existing conventions for issue and PR titles and format. This helps us maintain consistency and makes it easier for everyone to follow along.
- In future contributions, I kindly ask that you refrain from adding requests for specific labels like Hacktoberfest, as we add those based on contribution quality and alignment with project goals.
Thank you again for your effort, and I look forward to seeing the updated version!
Soyvor
Description: Loading Indicator:
Add a spinner or loader to indicate that the processing is happening (currently, only text is used). YouTube URL Validation:
You can implement client-side validation to check whether the URL is a valid YouTube video link before submitting the form. Progress Bar:
If the backend provides some progress updates during video processing, you can display a progress bar. Audio Preview:
Optionally add an audio player alongside the video player to allow users to listen to the audio without music (assuming this is the app’s purpose). Multiple Input Types:
Add functionality to support both YouTube URLs and direct video file uploads, providing more flexibility for users. Success Message and Video Details:
Display a success message with more details about the processed video (such as title, duration) after processing is completed. Styling and Accessibility:
Improve the styling of the page to enhance user experience and add better accessibility features (e.g., labels and ARIA roles for elements). Mobile Responsiveness:
Make sure the design adapts properly to smaller screen sizes by improving the mobile responsiveness (such as hiding the video player on small screens until processing is complete).