-
Notifications
You must be signed in to change notification settings - Fork 114
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 basic window control #670
base: main
Are you sure you want to change the base?
Conversation
ce36ad1
to
f5578f8
Compare
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'm happy with the additional signals; less sure about the new widget
masonry/src/widget/window_handle.rs
Outdated
|
||
/// A titlebar area widget. | ||
/// | ||
/// An area that can be dragged to move the window. |
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.
This needs some more documentation about how it's expected to be used.
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.
Done, I hope it's clear enough.
ctx.place_child(child, Point::ORIGIN); | ||
bc.constrain(size) | ||
} | ||
None => bc.max(), |
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'd expect this to impose a max height by default.
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.
See my reply below. This is supposed to be just a container, so users of this widget should wrap it in their titlebar custom widget. So, I think that just filling the constrain is the right thing to do.
fn layout(&mut self, ctx: &mut LayoutCtx, bc: &BoxConstraints) -> Size { | ||
match self.child.as_mut() { | ||
Some(child) => { | ||
let size = ctx.run_layout(child, bc); |
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.
Maybe similarly here, although I can see arguments for not doing so
fn paint(&mut self, _ctx: &mut PaintCtx, _scene: &mut Scene) {} | ||
|
||
fn accessibility_role(&self) -> Role { | ||
Role::GenericContainer |
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 suspect this is right, but I'm not certain
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 think it's right, since this is a container. The actual titlebar should have the Role::TitleBar
.
} | ||
} | ||
|
||
fn paint(&mut self, _ctx: &mut PaintCtx, _scene: &mut Scene) {} |
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.
Nothing drawn at all here feels a bit weird
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.
See my reply below. This is supposed to be just a container, so users of this widget should wrap it in their titlebar custom widget and do their drawing in there.
Well, I guess I should've included more information on how the The |
f5578f8
to
00b7aac
Compare
00b7aac
to
bb4d08f
Compare
This is the first merge-able part of #606.
It allows to implement custom CSDs via window management signals and a new
WindowHandle
widget which is useful to implement a custom titlebar.