Skip to content

Added possibility to inject canvas container for fullscreen#4369

Closed
zobertke wants to merge 1 commit intoemscripten-core:masterfrom
zobertke:fullscreen_external_container
Closed

Added possibility to inject canvas container for fullscreen#4369
zobertke wants to merge 1 commit intoemscripten-core:masterfrom
zobertke:fullscreen_external_container

Conversation

@zobertke
Copy link

No description provided.

@kripken
Copy link
Member

kripken commented May 31, 2016

First thing, we need prs to be to incoming, and this code changed recently, so it might need rebasing.

@kripken
Copy link
Member

kripken commented May 31, 2016

But maybe @juj wants to take a look before you do that work, for general feedback.

@juj
Copy link
Collaborator

juj commented Jun 2, 2016

I think I'd actually favor killing the library_browser code for entering and exiting fullscreen mode, and always using the HTML5 API to manage fullscreen, since that does not add/remove elements to the DOM, but only uses CSS styles to unify the per-browser differences that exist in fullscreen mode handling. Commented in #4370 about this, does the HTML5 API solve the issue for you?

The only trouble is that the HTML5 API is bloated due to not currently having efficient dead code elimination (PR #3298 attempts to address that, but I don't have a good solution to finish that yet), and that the HTML5 API is not exported to handwritten JS by default. Though I believe we'd like users to export the identifiers manually anyways.

@zobertke
Copy link
Author

zobertke commented Jun 2, 2016

Yes, actually that was the way to go. See #4370 (comment)
Thanks for the suggestion.

@zobertke zobertke closed this Jun 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants