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

Implement CloseRequested for Web #3311

Open
daxpedda opened this issue Dec 24, 2023 · 7 comments
Open

Implement CloseRequested for Web #3311

daxpedda opened this issue Dec 24, 2023 · 7 comments
Labels

Comments

@daxpedda
Copy link
Member

daxpedda commented Dec 24, 2023

This doesn't actually make sense currently, because Window on Web should be a view, see #696.
After we implement views for Web, and the Window actually represents the Web window, we could make CloseRequested a special event that if the user doesn't react to with EventLoopWindowTarget::exit(), we call Event.preventDefault().

This is useful to implement a confirmation dialog and the like, similar to what can be done on native by reacting to CloseRequested.

Firefox currently disables the B/F cache when registering the beforeunload event (Bugzilla, so until this addressed we should not implement it or disable it by default).

@madsmtm
Copy link
Member

madsmtm commented Feb 20, 2024

I disagree that Window should representing the browser window / the global window, it should represent a single HTML element as it does now.

Given that, I think this is better solved with a separate top-level event, like Event::CloseRequested. Such an event would also make sense on macOS as applicationShouldTerminate:.

@daxpedda
Copy link
Member Author

daxpedda commented Feb 20, 2024

I disagree that Window should representing the browser window / the global window, it should represent a single HTML element as it does now.

My motivation here is to make this align more with other platforms:

  • Multi-window is a thing in Web, its just not possible with Wasm. So calling it Window is a bit weird.
  • If a Window doesn't represent the browser window, how are we supposed to handle some of the things like title, outer_size(), icons and such things that only make sense for the the whole window and not a single HTML element.
  • A canvas that covers the whole browser window and a canvas that is only an embedded HTML element have often different use-cases and therefor should behave differently. This is aligns well with a Window vs View distinction on other backends.

Given that, I think this is better solved with a separate top-level event, like Event::CloseRequested. Such an event would also make sense on macOS as applicationShouldTerminate:.

That sounds like a good solution for the WindowEvent::CloseRequested problem on Web, but it doesn't cover all the other problems.

Apologies, I can see how my OP is incomplete now that I read it again. (EDIT: I think I left it out of OP because this is only about the CloseRequested event and not about if and how we should implement views for Web)

@kchibisov
Copy link
Member

I disagree that Window should representing the browser window / the global window, it should represent a single HTML element as it does now.

I don't think that it should be the case, I'd rather have Subview per element, I'm pretty sure web is rather limited in terms of APIs it has on a window that match other platforms, but it should much view pretty much?

Also how keyboard input is done? I'd assume it's for the main window, not for the HTML element in particular, but pointer events are likely HTML element aware?

Top-level should terminate is a bit questionable given that on web it looks a bit like window, but I don't really mind that.

@daxpedda
Copy link
Member Author

I disagree that Window should representing the browser window / the global window, it should represent a single HTML element as it does now.

I don't think that it should be the case, I'd rather have Subview per element, I'm pretty sure web is rather limited in terms of APIs it has on a window that match other platforms, but it should much view pretty much?

There are no subviews in canvases on Web, AFAIK. That's where the idea comes from: just make a canvas be a subview, which would also look like subviews on other platforms.

Also how keyboard input is done? I'd assume it's for the main window, not for the HTML element in particular, but pointer events are likely HTML element aware?

Every event should be HTML element aware, main window events are DeviceEvents.

Top-level should terminate is a bit questionable given that on web it looks a bit like window, but I don't really mind that.

I think I like @madsmtm suggestion so far, that is adding a Event::CloseRequested, which I think would be better in any case because Web can't actually close individual canvases. So basically WindowEvent::CloseRequested will be unimplemented on Web in general.

@madsmtm
Copy link
Member

madsmtm commented Feb 20, 2024

not about if and how we should implement views for Web

Apologies for hijacking the issue, though by now it's too late, so I'll just respond here anyhow ;)

Multi-window is a thing in Web, its just not possible with Wasm. So calling it Window is a bit weird.

That's fair, though I think that's more of a documentation issue.

title

Yeah, I can see this being annoying. A solution could be to allow referencing an element like a h1 that represents the title, such that set_title still works per window? Or the thing you do with the alt text currently also works fine.

window icons

These aren't really doable on most platforms, but I guess you could add set_icon as a platform-specific extension to EventLoop?

outer_size

I think I'd expect this to return canvas size + margin? It's not like you can really get the outer dimensions of the browser window anyhow?

(Not that I believe this definition of outer_size to be all that useful on the web, but probably fine, it isn't really all that useful in general).

A canvas that covers the whole browser window and a canvas that is only an embedded HTML element have often different use-cases and therefore should behave differently.

Hmm, can you expand on this?

@daxpedda
Copy link
Member Author

daxpedda commented Feb 20, 2024

not about if and how we should implement views for Web

Apologies for hijacking the issue, though by now it's too late, so I'll just respond here anyhow ;)

All good, thank you for the input!

title

Yeah, I can see this being annoying. A solution could be to allow referencing an element like a h1 that represents the title, such that set_title still works per window? Or the thing you do with the alt text currently also works fine.

Ofc, but how do you actually change the title of the browser window, which is also important and mimics other platforms much closer.

window icons

These aren't really doable on most platforms, but I guess you could add set_icon as a platform-specific extension to EventLoop?

Yes, I was hoping that one day we can expose it regularly at Window and just be no-op on non-supported platforms.

outer_size

I think I'd expect this to return canvas size + margin? It's not like you can really get the outer dimensions of the browser window anyhow?

It is possible, I just don't know how useful it is. See Window.outerWidth/Height.

(Not that I believe this definition of outer_size to be all that useful on the web, but probably fine, it isn't really all that useful in general).

Haha, yes, just watched you adding that :)

A canvas that covers the whole browser window and a canvas that is only an embedded HTML element have often different use-cases and therefore should behave differently.

Hmm, can you expand on this?

A lot of use-cases involve just having a canvas cover the whole screen, which we don't support very well, we expect users to fix it themselves, like setting CSS width: 100%; height: 100%.
In comparison embedded canvases don't want that.

Ofc we can fix all of that with platform-specific extensions, but it would be much nicer if we have a cross-platform API that doesn't require so much Web-specific code. The idea here is that Window vs View would map across platforms well.

@madsmtm
Copy link
Member

madsmtm commented Feb 20, 2024

Yeah, I can see how that makes sense, given that multiple windows aren't supported on Android either, I guess it's somewhat a fools errand to try to make the multiple windows model work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants