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: overlay layout for Responsive #1262

Merged
merged 1 commit into from
Feb 23, 2022

Conversation

nicksenger
Copy link
Contributor

@nicksenger nicksenger commented Feb 22, 2022

The layout node passed to a child of Responsive may be incorrect in some cases. This PR fixes the overlay.

@hecrj hecrj added the bug Something isn't working label Feb 22, 2022
@hecrj hecrj added this to the 0.4.0 milestone Feb 22, 2022
@@ -258,7 +258,7 @@ where
None => self.body.overlay(children.next()?, renderer),
}
} else {
self.body.overlay(layout, renderer)
self.body.overlay(layout.children().last()?, renderer)
Copy link
Member

@hecrj hecrj Feb 22, 2022

Choose a reason for hiding this comment

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

This seems incorrect, since we are returning the body layout directly in layout when no title bar is present:

} else {
self.body.layout(renderer, limits)
}

Copy link
Contributor Author

@nicksenger nicksenger Feb 22, 2022

Choose a reason for hiding this comment

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

Oh you're right 🤔, not sure why this fixes the cases I'm looking at then. There must be an extra layout node coming in somewhere else.

My bad, what I was seeing actually has nothing to do with PaneGrid and was due to the issue with Component's overlay that was already fixed in #1205.

Updated this to remove the PaneGrid changes.

.unwrap()
.borrow_overlay()
.as_ref()
.map(|overlay| overlay.position())
Copy link
Member

Choose a reason for hiding this comment

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

Ah, good catch!

@nicksenger nicksenger changed the title Fix: overlay layout for PaneGrid & Responsive Fix: overlay layout for Responsive Feb 22, 2022
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants