-
Notifications
You must be signed in to change notification settings - Fork 605
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
Add always-on-top
property to Window element
#2557
Add always-on-top
property to Window element
#2557
Conversation
if (size != widget_ptr->size()) { | ||
widget_ptr->resize(size.expandedTo({1, 1})); | ||
} | ||
widget_ptr->setWindowFlag(Qt::FramelessWindowHint, no_frame); | ||
widget_ptr->setWindowFlag(Qt::WindowStaysOnTopHint, always_on_top); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found in this list:
https://doc.qt.io/qt-6.2/qtwidgets-widgets-windowflags-example.html
winit_window.set_window_level(if window_item.always_on_top() { | ||
winit::window::WindowLevel::AlwaysOnTop | ||
} else { | ||
winit::window::WindowLevel::Normal | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Does being fullscreen matter like it does for no-frame
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it matters :)
in property <bool> no-frame; | ||
in property <bool> always-on-top; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the ordering arbitrary, here? In the docs, the ordering is alphabetical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. Even the alphabetical ordering in the docs I would personally rather group by relevance. This looks good to me.
@@ -1174,6 +1174,7 @@ pub struct WindowItem { | |||
pub background: Property<Brush>, | |||
pub title: Property<SharedString>, | |||
pub no_frame: Property<bool>, | |||
pub always_on_top: Property<bool>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There aren't many window items around, so I think this is fine, too.
Looks good to me. Let's see what the others say :) |
Very good PR, complete with documentation and changelog entry. There are no test but I can't think how this could be tested. Thank you for the contribution. |
It's a very simple implementation, which could be made by mostly pattern matching the existing code path for no-frame and no_frame. I've added my own review for things that stood out as potential questions.