Wasm Improvements

[NwE0kmCE]

This PR makes some changes to how Windows are instantiated on wasm targets

• The title parameter in Window::new() has been replaced with container, which takes an element the canvas should be appended to
  • Allows more control over where and how to embed a minifb canvas in web projects, and still allows the user to change the window title with set_title
  • Rustdoc new window example has been updated for wasm32 target
• set_cursor_visibility now properly sets the the html canvas cursor style
• is_active now checks if the canvas element is focused

[NwE0kmCE]

To avoid a breaking API change, title could be used to select a dom element by id...

[emoon]

Hey,

Thanks for the PR! I wonder if it's possible to include an example that works with these changes also? It would be nice to keep the API same between desktop and wasm, but if it doesn't make sense from a web perspective I'm fine with the change.

[NwE0kmCE]

Hi! Sorry for the delay! Working on the example now, and pretty quickly I'm running into a case where having a unified API would be better: all non-web examples fail to compile when the target is set to wasm... I do think that on web the page title shouldn't be changed, since most of the time (i assume) someone would be embedding minifb into a larger project/site where the page title is already defined somewhere else. I think passing the id of a dom element works, as long as there's some sign (e.g. rustdoc comment) that the behavior is different between desktop & web

[NwE0kmCE]

Also, I noticed that the web feature is only used in 2 places (rate.rs and key_handler.rs) and could probably be replaced with a target = "wasm32" attribute, would it make sense to replace those and remove the web feature?

[emoon]

Also, I noticed that the web feature is only used in 2 places (rate.rs and key_handler.rs) and could probably be replaced with a target = "wasm32" attribute, would it make sense to replace those and remove the web feature?

Sounds good to me

[NwE0kmCE]

This should be ready to go!

[emoon]

Great! Thanks.

I wonder how hard would it be to add a build step for this in https://github.com/emoon/rust_minifb/blob/master/.github/workflows/ci.yml ? Just to make sure it doesn't break when doing other changes.

[NwE0kmCE]

I think this change should work... Building is no problem, but testing on wasm requires a whole bunch of extra tooling since the generated wasm isn't an executable on its own. Since the only tests being run anyways are doc tests this should be fine:

    - name: Build
-     run: cargo build --verbose
+     run: |
+          cargo build --verbose
+          cargo build --verbose --target wasm32-unknown-unknown 
    - name: Run tests
-     run: cargo test --verbose
+     run: |
+          cargo test --verbose
+          cargo test --doc --verbose --target wasm32-unknown-unknown 

more info on wasm testing here

[emoon]

Thanks for the PR. I will look into getting the build step for the CI running in another change.