search

Kagami / boram

:film_strip: Cross-platform graphical WebM converter
https://github.com/Kagami/boram/releases
433 stars 30 forks source link

Show progress bar in taskbar #29

Closed emyarod closed 6 years ago

emyarod commented 6 years ago

This PR will introduce a progress bar in the taskbar of OSes that support this Electron integration, allowing the Electron window to provide progress info without the window being focused.

Theoretically, this should be in macOS and the Unity DE as well, but I have not been able to test that yet (would like help with that, if possible). Here is an example of it working in Windows, though:

Windows 10 example

Kagami commented 6 years ago

It should be handled somewhere in main-tabs/index.js:handleProgressChange, also need some way to handle multiple progresses (because there might be multiple tabs), maybe simple mean value would be fine. And also it should be explicitly cleared to -1 according to the docs.

emyarod commented 6 years ago

One question regarding handling multiple progress values: is there a way for me to figure out whether or not a tab is actively encoding? I think ideally we should avoid inactive tabs in the mean progress value. Should we place a property on each tab object stating whether or not an operation is active? or is that information already available somewhere?

Kagami commented 6 years ago

See https://github.com/Kagami/boram/blob/master/src/main-tabs/index.js#L138-L142 Each tab has progress property associated (0=off, 1-99=some operation is in progress).

emyarod commented 6 years ago

oh okay I was just unsure if I could assume that 0 progress meant inactive. thanks!

emyarod commented 6 years ago

I've made your requested changes, please let me know what you think

On a side note, the application console displays an error message caused by the invalid tabKey prop on L237 and L255. Is that prop in use or should it be removed?

Kagami commented 6 years ago

Hi, sorry for the delay. This looks good. Maybe we should set progress bar of window with remote.getCurrentWindow() right away though? It looks a bit simpler.

On a side note, the application console displays an error message caused by the invalid tabKey prop

Seems like some props were renamed/removed from material-ui component. I will take a look.

Kagami commented 6 years ago

On a side note, the application console displays an error message caused by the invalid tabKey prop

Ah, actually because you need to apply patch:

patch -p1 < mui-tabs.patch

from the repository root. This is awful, I need to PR this but hadn't enough time.

emyarod commented 6 years ago

Maybe we should set progress bar of window with remote.getCurrentWindow() right away though? It looks a bit simpler.

I've gone ahead and made that change. I am still unfamiliar with Electron's API so thank you for the correction.

I also added a postinstall script to package.json to patch Material UI automatically, would that work?

Kagami commented 6 years ago

Merged, thanks!

would that work?

According to docs, should work. I will try to PR/fork repo to avoid such ugly hacks though. There is also epackager-path.patch btw.