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

Multi-window #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

derezzedex
Copy link
Member

This proposal aims to introduce the concept of multi-windowed applications to iced, while conforming as best as possible to its current architecture and goals.

Rendered

@hecrj hecrj added the enhancement New feature or request label Aug 31, 2022
@13r0ck
Copy link
Member

13r0ck commented Sep 1, 2022

Seems awesome to me! I love that this approach allows for the developer to choose the level of complexity they want for their app.

Comment on lines +88 to +90
fn windows(&self) -> Vec<(window::Id, window::Settings)>{
// ...
}
Copy link
Member

Choose a reason for hiding this comment

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

As we discussed in the monthly meeting a while ago, the only concern I have with this RFC is the design of this windows method.

Particularly, it seems misleading to ask for window::Settings since we can't always control them in the runtime (size and position being clear examples).

Maybe we can just ask for the titles of the windows here (i.e. a String) and allow modifying window settings through commands.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that window::Settings might not ideal, but I really like the fact that the API simplifies a lot by making user code manage the windows.

I think that if you change the windows method to only contain the title, whenever you spawn a window there will be a brief moment where it's in the wrong state (waiting for the Command with the correct window::Settings).

Would it be better to add window::Id to Application::title and then note on the Application::windows that those are "spawning" settings for the window, and that further changes need to be done through Action::Window?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've started testing the window::Action::Spawn and window::Action::Close route.
The main downside of this is that window::Action is not exclusive to multi_window, so maybe we have to do something similar to Action::System?

Choose a reason for hiding this comment

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

@derezzedex I just read the RFC, and I came here to suggest window::Action::Spawn and window::Action::Close. I think it makes sense to make window creation/destruction imperative due to it being a relatively rare occurance. Might it be reasonable to make every application a "multi window" application, but have window::Id(1) created automatically by Iced (and perhaps exported as constant called DEFAULT_WINDOW or MAIN_WINDOW or similar)?

If I understand thing correctly, the only user-facing change would be the introduction of the window parameter to the view method in the Application trait. And creators of single-window applications could simply ignore this parameter. That doesn't seem very intrusive to me, and this parameter could be elided from the Sandbox trait used by very simple applications.

@hecrj
Copy link
Member

hecrj commented Dec 2, 2023

It'd be cool to eventually update the RFC to document the implementation, as there have been many undocumented changes in the process—although synchronized with the core team through other means.

No rush on that, though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants