Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multiple systrays on standalone Mac and Linux caused by window.location.reload() #569

Closed
Js41637 opened this issue Oct 8, 2015 · 15 comments
Assignees

Comments

@Js41637
Copy link
Collaborator

Js41637 commented Oct 8, 2015

From reddit; https://www.reddit.com/r/duckietv/comments/3nwzct/system_tray/
Currently the right click context menu (in windows 10 anyway) is empty. Would be useful to have a close/exit option there instead of having to maximize the window again.

Js41637 added a commit that referenced this issue Oct 8, 2015
Added a prototype code for checking if database operations are ongoing.
see comments for details.
@garfield69 garfield69 added this to the Roadmap v1.2.0 milestone Oct 8, 2015
@Js41637 Js41637 changed the title Exit option in standalone systray menu Exit + others option in standalone systray menu Oct 9, 2015
@Js41637
Copy link
Collaborator Author

Js41637 commented Oct 9, 2015

Exit button has been added and SchizoDuckie wondered if we could add others. I think we could add most of the major links to the tray, Library, Settings & About

garfield69 added a commit that referenced this issue Oct 15, 2015
Items for calendar, favorites, settings, and about added.
on_click routines for each set up.
next step, figure out how to add navigation commands into each item's click routines.
At the moment all the menu items just restore window.
garfield69 added a commit that referenced this issue Oct 16, 2015
@garfield69
Copy link
Collaborator

please test this, especially on standalone Mac, checking that tray duckies still behave, that the menu works, etc. thanks.
Tested on win32 standalone, seems to work for me.

@lonelyfairie
Copy link

With latest comit I am seeing multiple duckies again when minimizing to tray :(
screen shot 2015-11-08 at 14 23 23

They don't multiply as before, but I see two all the time.

garfield69 added a commit that referenced this issue Nov 8, 2015
please test on mac and report if it works thanks.
@lonelyfairie
Copy link

looks ok now. 👍

@lonelyfairie
Copy link

Mmm it seems it doesn't happen instantly but rather after some time having duckie tv open, this morning the 2 duckies where back:

screen shot 2015-11-10 at 10 17 59

After restarting the computer (unrelated to duckie) now I only have one duck on the sys tray.

@garfield69
Copy link
Collaborator

I wonder if DuckieTV is creating a new tray while it is minimized to systray during the local midnight reload that DuckieTV does to refresh the calendar after the date change.

garfield69 added a commit that referenced this issue Nov 10, 2015
@SchizoDuckie
Copy link
Owner

That's very possible. There's a hard window.location.reload there. Maybe we
should really look Into injecting those scripts with a node-webkit load
event instead of on regular window onload
Op 10 nov. 2015 18:41 schreef "garfield69" [email protected]:

I wonder if DuckieTV is creating a new tray while it is minimized to
systray during the local midnight reload that DuckieTV does to refresh the
calendar after the date change.


Reply to this email directly or view it on GitHub
#569 (comment)
.

garfield69 added a commit that referenced this issue Nov 10, 2015
@garfield69
Copy link
Collaborator

@lonelyfairie If you are willing and have the time, I have a test I would like performed on a Mac DuckieTV standalone, to test a theory as to the cause of the systray duck cloning.
After downloading the latest DuckieTV standalone, click on the About icon (i) on the left bottom of the main DuckieTV page. Scroll to the bottom and you should find a Mac Reload Systray Test section.
Follow the instructions there and report back at your convenience.
If you are unable to perform the test, then don't worry about it, as it can wait until Schizoduckie has time to do it instead. Cheers.

@lonelyfairie
Copy link

@garfield69 yeap, two duckies appear after the sys tray reload, if you disable and enable it again a third duckie appears... and so on to infinity, well untill the tray fills up lol.

moar duckies

@garfield69
Copy link
Collaborator

@lonelyfairie We've made some further changes and cleaned up some logic holes.
Please test the latest development source (or the 20151209 nightly version) on your Mac when you have the time. Use the Mac Reload Systray Test if you need to. Thanks.

@lonelyfairie
Copy link

@garfield69 Just tested this version, still seeing multiple duckies when using the Mac Reload Systray Test otherwise it seems to be running correctly, no extra duckies when just minimizing, no issues with the window not showing up or exiting the app by using the context menu Quit.

garfield69 added a commit that referenced this issue Dec 11, 2015
@garfield69
Copy link
Collaborator

@lonelyfairie allright, that is good to know.
so in summary, the systray logic on mac seems to be working properly, but we still have the outstanding issue of a window.location.reload() causing duckietv problems.
as Schizoduckie said earlier,

Maybe we should really look Into injecting those scripts with a node-webkit load event instead of on regular window onload

so that is what we will be looking into next.

@garfield69 garfield69 changed the title Exit + others option in standalone systray menu multiple systrays on standalone Mac and Linux caused by window.location.reload() Jan 15, 2016
@garfield69
Copy link
Collaborator

Schizoduckie previously said:

Maybe we should really look Into injecting those scripts with a node-webkit load event instead of on regular window onload

On the assumption he meant using the nwjs Window.reload() in place of the hard window.location.reload(), I set up a proof of concept test on my ubuntu 15.10 duckietv standalone development system (since the multi-systray issue occurs on both mac and linux).

I set up the following code with app.js

/**
 * at start-up set up a timer to refresh DuckieTV a second after midnight, to force a calendar date refresh
 */
.run(
    window.onload = function() {
        var today = new Date();
        var tommorow = new Date(today.getFullYear(), today.getMonth(), today.getDate() + 1);
        var timeToMidnight = (tommorow - today) + 1000; // a second after midnight
        // #569 test
         if (localStorage.getItem('mac_systray_reload_test')) {
            timeToMidnight = 10000; // 10 second reload test for mac systray
         };
         // end #569 test
        var timer = setTimeout(function() {
            if ((navigator.userAgent.toLowerCase().indexOf('standalone') !== -1)) {
                console.debug('nw.gui.Window.get().reload()');
                require('nw.gui').Window.get().reload();
            } else {
                console.debug('window.location.reload()');
                window.location.reload();
            };
        }, timeToMidnight);
    }
)

I then enabled my reload test for every 10 seconds, and clicked on the minimize button every time the window got reloaded.

screencapture:
image

As the screencapture shows, my test using nwjs reload in this configuration did not solve the multi-systray issue.

@SchizoDuckie did I use the correct function for this test, or did you mean something else?

@Js41637
Copy link
Collaborator Author

Js41637 commented Jan 15, 2016

Here's an idea, why dont you destroy the tray before reloading

@garfield69
Copy link
Collaborator

@Js41637 💡 👍
I'll work on this tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants