zumoshi / BrowserSelect

Browser Select is a utility to dynamically select the browser you want instead of just having one default for all links.
GNU General Public License v2.0
289 stars 39 forks source link

Adding refresh button and caching browsers. #53

Closed kthejoker closed 5 years ago

kthejoker commented 5 years ago

Whoops, thought I was doing a PR to my own master ...

Anyway, this caches the user's browser list in the settings and only refreshes when the user goes into settings and hits the "Refresh" button (also in PR) next to the browser list.

This reduces startup time of the app by ~80% (10 run avg, 3 Chrome profiles, 2 FF profiles, IE and Edge: 4.06 seconds vs 0.88 seconds).

Possible improvement: staling out the cache over some period of time to occasionally automatically check for browsers/profiles.

zumoshi commented 5 years ago

I tested it and it is even faster than I thought. The change from the first run (without cache) to second was very noticeable. we can close #40 by merging this.

I only have one complaint, If you had free time to work more on it, If not it's fine. and that is lack of any tooltip/help text. I think users may be confused with how it works. especially since after clicking refresh and closing settings, it doesn't update the list of browsers.

Maybe add a message box after the cache has been invalidated that says List of browsers updated. new browsers will show up on the next start or if you had a lot of free time maybe make it refresh the main screen when settings dialogue is closed so changes would apply without needing to restart the app?

kthejoker commented 5 years ago

Hmm - absolutely, I see your point. Yes, I will work on this and have it refresh the main form as well.

kthejoker commented 5 years ago

Okay, I implemented so that when a user hits refresh the main form updates with the new browser list. Some TODO improvements:

And of course more generally we should have a BindingSource between our browser list and the controls so we can just remove items from the list and have the controls update automatically. Some other day ...

zumoshi commented 5 years ago

Neat.

I don't think making a binding is worth it given that this would only be used when a new browser is installed/removed. I would've done it the same way. But I would maybe use a SuspendLayout call to prevent the in-between state:

image (this is shown for maybe 2-3 secs with new browsers showing one by one)

zumoshi commented 5 years ago

I just noticed a side effect you caused with your last update, this.Controls.Clear(); removes btn_help but doesn't re-add it. image

But don't worry about it I'll fix it. Thanks again for your work, I'll try to put a release out tonight after fixing #52 .