search

gurumukhi / youtube-screenshot

Cross browser addon for youtube video screenshots
https://addons.mozilla.org/en-US/firefox/addon/youtube-screenshot-button/
Mozilla Public License 2.0
41 stars 8 forks source link

Hot reload configuration #32

Closed joggee-fr closed 1 year ago

joggee-fr commented 1 year ago

This PR mainly add hot reload mechanism when changing add-on preferences. It avoids to refresh the opened YouTube page to apply newly selected options. It works sending message from options script to tabs containing content script. The configuration is then reloaded when receiving such a message.

I have done some tests but of course should be validated by other users here. Thanks.

gurumukhi commented 1 year ago

Thanks for taking it up @joggee-fr :)

Code changes looks good to me. I wont have time to test it myself for some time, but lets hope if someone else can test it well.

gurumukhi commented 1 year ago

Also, I hope these changes will work well with our future idea of moving options to toolbar popup dialog (https://github.com/gurumukhi/youtube-screenshot/issues/23).

joggee-fr commented 1 year ago

Also, I hope these changes will work well with our future idea of moving options to toolbar popup dialog (#23).

I did not check yet how to create toolbar popup for add-ons. However, according to documentation, the used mechanism to send message to content script is usable from all background scripts whatever is it from options or toolbar popup.

I hope someone here could try this PR...

joggee-fr commented 1 year ago

@PRIXYY, as you have been active recently on an other issue... maybe you could give my PR a try 😄 .

joggee-fr commented 1 year ago

I have just fixed an issue with logger not able to be disabled using hot reload. Anyone to accept or reject my PR 😥 ?

PRIXYY commented 1 year ago

I am currently testing your changes joggee

PRIXYY commented 1 year ago

I have tested the addon and so far there has been no bugs ,I have tested it on different quality ,full screen ,in incoginto ,etc.Seems to be working fine.

joggee-fr commented 1 year ago

Rebased on current master.

gurumukhi commented 1 year ago

Thanks for amazing work @joggee-fr. Sorry for delay but I am quite busy with some other commitments actually. Since this PR contains major changes I wanted to review & test it thoroughly before releasing it. I am planning to take it up as first thing in next quarter. Sorry about the delay again and kudos for your efforts.

gurumukhi commented 1 year ago

Hi @joggee-fr, we can now release next big version (4.0.0). Can you please resolve the conflicts of this branch and also check other PRs and issues and see which all can be knocked off in this release?

joggee-fr commented 1 year ago

Replaced by PR #48.