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

Make windows use native window decorations by default #526

Merged
merged 1 commit into from
Mar 11, 2023

Conversation

zjeffer
Copy link
Collaborator

@zjeffer zjeffer commented Mar 11, 2023

Users have reported (#521) that it would be better if Windows used native window decorations by default. This PR sets that.

(not tested yet but I assume this small change works fine)

Closes #525

@guihkx
Copy link
Collaborator

guihkx commented Mar 11, 2023

In my opinion, we should not be changing options set by the user (as some may prefer custom decorations, for example).

A safer approach, in my opinion, would be using native window decorations by default when our config file is not present (i.e., it's the first time the program is running).

@zjeffer
Copy link
Collaborator Author

zjeffer commented Mar 11, 2023

A safer approach, in my opinion, would be using native window decorations by default when our config file is not present

That's what this PR does, no?

@guihkx
Copy link
Collaborator

guihkx commented Mar 11, 2023

Oh, you're right. The second parameter of value() sets the default one.

In any case, I'll test this shortly.

Copy link
Collaborator

@guihkx guihkx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works on Windows 7!

A minor styling choice nitpick: I'd use #if defined(Q_OS_WIN) instead of #ifdef _WIN32.

Other than that, LGTM.

@zjeffer
Copy link
Collaborator Author

zjeffer commented Mar 11, 2023

Thanks for testing!

@zjeffer zjeffer merged commit 0421737 into master Mar 11, 2023
@guihkx guihkx deleted the windows-native-by-default branch March 11, 2023 15:40
@zjeffer
Copy link
Collaborator Author

zjeffer commented Mar 11, 2023

@nuttyartist On the get-notes.com homepage, under the "Cross-platform" header there's a screenshot for every platform. I think we should update the Windows screenshot with one using native decorations now.

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.

Make native window decoration the default on Windows
2 participants