khrykin / StrategrDesktop

No-Fuss Time Management App
https://khrykin.github.io/strategr/
GNU General Public License v3.0
312 stars 34 forks source link

opening a .stg file from finder creates 2 windows #7

Closed borco closed 4 years ago

borco commented 4 years ago

In Finder, double-click on an .stg file. The app will open it, but it will also open a window with title Untitled (with no activities), as in the screenshots bellow.

Screenshot 2020-02-08 at 18 01 13

Screenshot 2020-02-08 at 18 00 16

khrykin commented 4 years ago

@borco Thank you for reporting, unfortunately, I can't reproduce this is on my machine with the latest release (v0.0.7). Can you please give more context, macOS version?

borco commented 4 years ago

macOS Catalina: 10.15.3 (19D76) Xcode 11.3.1 (11C504) Qt: 5.14.1 (using online installer from https://qt.io) boost, nlohmann_json, utf8Proc, catch2 installed with brew

khrykin commented 4 years ago

@borco Can you find out which minimal timeout here would resolve the issue on your system?: https://github.com/khrykin/StrategrDesktop/blob/a669ff5d7f21f5e1e79d2b87c391ecbbcd0a1b0f/ui/application.cpp#L19

borco commented 4 years ago

Using QTimer for this doesn't look right to me.

Instead, I would:

something like:

class Application ... {
...
signals:
  void loadStrategy(const QString&); // emitted when clicking on .stg in Finder
...
}

class MainWindow ... {
...
public:
  void onLoadStrategy(const QString&); // can replace loaded stg with another one
...
}

main() {

...
QObject::connect(&a, &Application::loadStrategy, &window, &MainWindow::onLoadStrategy);
...
}
borco commented 4 years ago

Btw, I've looked again in your code and you're actually creating the window inside your custom Application class - so you should probably move that connect there...

khrykin commented 4 years ago

@borco It was done this way in the past, but I introduced QTimer to eliminate "blinking" when the default strategy was visible for a split second if opening from a file.

borco commented 4 years ago

Yes, blinking might happen - hanven't thought about it.

Have you tried to load the default strategy when the window is shown the first time if other strategy is not already loaded? Haven't tested it myself, but the signal from Finder might come before the main window is actually shown...

QTimer definitely doesn't look right to me, but I guess you can use it until you find a better strategy...

khrykin commented 4 years ago

@borco QFileOpenEvent fires after QApplication starts, so there is no way of knowing beforehand if the app was started by clicking on a file or just by itself on macOS. One way or another - we still need to wait for some time to check if QFileOpenEvent actually has been fired, if we don't want the blinking.

borco commented 4 years ago

Than just put some msec (like 500msec) in QTimer... I'm curious how a native Swift/Objective-C app would handle this...

khrykin commented 4 years ago

@borco there is 100ms in v0.0.9 , can you please check the new version? Native apps can have application:openFile: in the app's delegate and you can handle this situation appropriately, but unfortunately, as far as I know, in Qt there is no applicationDidFinishLaunching: event analog.

borco commented 4 years ago

Would this help? https://wiki.qt.io/Application_Start-up_Patterns

khrykin commented 4 years ago

I think using native event loop and manually hooking Qt into it would be a "killing a fly with a sledgehammer" type of solution for this particular problem. 😀

borco commented 4 years ago

master isn't building for me.

[ 51%] Building CXX object CMakeFiles/ui_tests.dir/ui/macoswindow.mm.o
/Users/borco/Developer/StrategrDesktop/ui/macoswindow.mm:13:10: fatal error: 'QtMac' file not found
#include <QtMac>
         ^~~~~~~
1 error generated.
make[2]: *** [CMakeFiles/ui_tests.dir/ui/macoswindow.mm.o] Error 1
make[1]: *** [CMakeFiles/ui_tests.dir/all] Error 2
make: *** [all] Error 2
19:16:07: The process "/usr/local/Cellar/cmake/3.16.2/bin/cmake" exited with code 2.
Error while building/deploying project Strategr (kit: Desktop Qt 5.14.1 clang 64bit)
When executing step "CMake Build"

this was one of the things fixed in my pull request, but that can no longer be applied as your code changed.

btw, your project looks interesting, but unfortunately i can't dedicate this much time to it. please feel free to close my tickets as you feel appropriate. br.

khrykin commented 4 years ago

I'll fix this thank you, "Strategr" target should build, however.