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 implementation for Lazy widget #1644

Merged
merged 4 commits into from
Jan 11, 2023

Conversation

nicksenger
Copy link
Contributor

@nicksenger nicksenger commented Jan 11, 2023

Fixes #1583

Currently the Lazy widget in iced_lazy is broken because we assume we can get_mut an Rc which is not unique - both the element & widget tree hold references to this data.

Since we now need a mutable reference to self in overlay to support operations, the compiler will prevent us from producing an overlay with a mutable reference to the same element that is being held by the Lazy & its widget tree node. As a workaround, I've temporarily removed the element from the references held on the Lazy & in the widget tree while the Overlay is in use, and restored it during the overlay's drop. Note that this will break if any widget methods from the parent widget are invoked prior to the overlay being dropped. Currently UserInterface::update is written such that this doesn't happen, but I'm not sure if it's necessarily guaranteed by Iced.

I've also added pick_lists to each item in the Lazy example to demonstrate that the overlays are working properly.

@tarkah
Copy link
Member

tarkah commented Jan 11, 2023

Looks good! Thanks for fixing this. Option ftw. I hate how overly complex things are due to ouroboros but not sure we can get around it w/ these use cases.

Example tested and working on Linux.

@hecrj
Copy link
Member

hecrj commented Jan 11, 2023

Note that this will break if any widget methods from the parent widget are invoked prior to the overlay being dropped.

Since overlay borrows mutably from the parent widget, this should be impossible because of borrow rules.

@hecrj hecrj added bug Something isn't working widget labels Jan 11, 2023
@hecrj hecrj added this to the 0.7.0 milestone Jan 11, 2023
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.

Looks great! Thanks!

@hecrj hecrj merged commit ca337b8 into iced-rs:master Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working widget
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lazy example broken
3 participants