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

fix #1144, focus window on macOS #1183

Closed
wants to merge 2 commits into from

Conversation

jinjiaodawang
Copy link
Contributor

Fix #1144: In macOS, when opening sioyek in full-screen mode, opening a file can still focus on the window

@jinjiaodawang jinjiaodawang mentioned this pull request Oct 9, 2024
@ahrm
Copy link
Owner

ahrm commented Oct 9, 2024

Can you see why the original focus code here does not work?

if (!nofocus) {
.

Your code doesn't handle some cases (e.g. when --nofocus is passed as an argument).

@jinjiaodawang
Copy link
Contributor Author

It seems that when running ./sioyek file.pdf --nofocus, this part of the code ensures that the window does not gain focus. However, when I open the file from the file browser, the code will directly run the handle_args function by emitting file_ready through OpenWithApplication.
(

sioyek/pdf_viewer/main.cpp

Lines 848 to 850 in 7181c5a

QObject::connect(&app, &OpenWithApplication::file_ready, [&main_widget](const QString& file_name) {
handle_args(QStringList() << QCoreApplication::applicationFilePath() << file_name);
});
)
In other words, the judgment of --nofocus will only be invoked when running in the terminal.
But you are correct that --nofocus is ignored.

@jinjiaodawang
Copy link
Contributor Author

We could move the judgement of --nofocus inside handle_args. You may reject my PR, or I fix it in that way if you perfer.

@ahrm
Copy link
Owner

ahrm commented Oct 9, 2024

We should probably handle it inside file_ready after running handle_args (not inside handle_args). Thanks for your efforts.

@ahrm ahrm closed this Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants