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

Backport emscripten-core/emscripten#7631 for focus on iframes #2307

Merged
merged 4 commits into from
Aug 30, 2020
Merged

Backport emscripten-core/emscripten#7631 for focus on iframes #2307

merged 4 commits into from
Aug 30, 2020

Conversation

fdelapena
Copy link
Contributor

@fdelapena fdelapena commented Aug 27, 2020

Multiple games on sites like rmarchiv itch.io and similar will benefit on this.
Additinally, remove the default 'Downloading...' text to prevent user confusion when robots show web content preview.

Closes: #804.

@fdelapena fdelapena added UX For issues affecting the user experience, such annoyances, counter-intuitive or ugly design Emscripten WebAssembly/JavaScript port for web browsers labels Aug 27, 2020
@fdelapena fdelapena added this to the 0.6.3 milestone Aug 27, 2020
@fdelapena fdelapena changed the title Backport emscripten-core/emscripten#7484 for focus on iframes Backport emscripten-core/emscripten#7631 for focus on iframes Aug 27, 2020
Copy link
Member

@Ghabry Ghabry left a comment

Choose a reason for hiding this comment

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

looks fine but can't be tested right now because of EasyRPG/buildscripts#83

@fdelapena fdelapena closed this Aug 28, 2020
@fdelapena fdelapena deleted the emscripten-focus-fixes branch August 28, 2020 17:32
@fdelapena fdelapena restored the emscripten-focus-fixes branch August 28, 2020 17:33
@fdelapena fdelapena reopened this Aug 28, 2020
@fdelapena fdelapena added the Awaiting Rebase Pull requests with conflicting files due to former merge label Aug 28, 2020
Multiple games on sites like rmarchiv itch.io and similar will benefit on this.
Additinally, remove the default 'Downloading...' text to prevent user confusion when robots shows web content preview.
@fdelapena fdelapena removed the Awaiting Rebase Pull requests with conflicting files due to former merge label Aug 28, 2020
@fdelapena
Copy link
Contributor Author

This is ready. Tested with Firefox and Chromium, automatic canvas resizing on windowed mode seems to work, though full screen toggle goes black for some (likely unrelated) reason.

@Ghabry
Copy link
Member

Ghabry commented Aug 28, 2020

Imo removing downloading is not the way to go. You could call it "Loading game..." and then use open graph standard to get a better link preview.

@fdelapena
Copy link
Contributor Author

fdelapena commented Aug 28, 2020

The "Downloading..." is set just later from JavaScript, so the experience doesn't change in practice. It could even contain a <noscript> there with meaningful Player details. Even the noscript gets overwritten from JS. Unfortunately not every preview out there uses open graph, including some search engines.

Copy link
Member

@Ghabry Ghabry left a comment

Choose a reason for hiding this comment

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

No idea how but your media queries (?) broke the fullscreen function.

You could also try with Module.requestFullscreen I replaced this with Module['canvas'].requestFullscreen because the emscripten solution was broken (it works by wrapping the canvas in a div and requesting fullscreen from the div)

@Ghabry
Copy link
Member

Ghabry commented Aug 30, 2020

There is this OBJECT_DEPENDS stuff in the CMake file where pre and post JS are specified. Just add the shell there and it will work

set_source_files_properties("src/main.cpp" PROPERTIES OBJECT_DEPENDS "${PLAYER_JS_PREJS};${PLAYER_JS_POSTJS};${PLAYER_JS_SHELL}")

@Ghabry Ghabry added the backport to stable Pull requests with changes backported to our stable, maintenance branch label Aug 30, 2020
@Ghabry Ghabry merged commit d5808d4 into EasyRPG:master Aug 30, 2020
@fdelapena fdelapena deleted the emscripten-focus-fixes branch August 31, 2020 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport to stable Pull requests with changes backported to our stable, maintenance branch Emscripten WebAssembly/JavaScript port for web browsers UX For issues affecting the user experience, such annoyances, counter-intuitive or ugly design
Development

Successfully merging this pull request may close these issues.

Emscripten: Update HTML shell file from newer releases
3 participants