leerob / youtube-to-mp3

⚡️Electron application to convert and download YouTube videos as MP3s
https://electronjs.org/apps/youtube-to-mp3
MIT License
540 stars 132 forks source link

Queue videos feature! #7

Closed IT-MikeS closed 6 years ago

IT-MikeS commented 6 years ago

Here is what I have come up with for queuing videos. I've tested it with 1,2,4,6,12,20, and 50 queued files thus far. ( just used same 3 links of mine in varying order)

Also hooked up a bitrate setting in preferences menu.

IT-MikeS commented 6 years ago

Switched to webpack over browserify.

This allows; scss, img assets and jsx to all be bundled closer together, only one npm task being needed to watch for changes, more clear file locations, less total files, better minification, and the expand-ability of webpack.

leerob commented 6 years ago

Definitely like the changes from using Webpack instead! Do we still need all the dev dependencies listed?

I'm not sure about the tab view - I see the idea you're going for, but I'm thinking it's making things too complicated. I tried testing locally and I couldn't get any videos to download. It put one in progress and then never started.

My suggestion would be to limit this PR to changing to webpack, and then adding the menu option for changing the bitrate. I'm not huge fan of the smalltalk UI. Maybe we can utilize a selection in the actual menu? e.g.

Select Target Bitrate - > 320 
                          256 
                          128 

That way you wouldn't even need a UI, and the newly selected value would get passed along to the front end.

I'm not totally ruling out the idea of a queue, I just think it will need to UI/UX thought first before implementing it. I think having a separate PR would help as well. Does that work for you?

IT-MikeS commented 6 years ago

Okay I'll swap to a new branch for the UI/UX changes with queue, I am currious to know what happened though with it not working for you, did you do npm i then build using the npm task webpack?

Smalltalk was just a quick idea I figured it may not survive in the app lol, I switch to the menu idea and see what it looks like.

Going to close this PR so that it easier and more clear as to what PR has what going on.