Skip to content

Conversation

@ditman
Copy link

@ditman ditman commented Feb 22, 2024

This Draft PR is a small example of a conversation that @diegotori and I had in another one.

This PR:

  • Adds a way for the plugin to await until JS is ready before calling things on it.
  • Simplifies enabled with the promiseToFuture helper.
  • Updates importJsLibrary so it returns a Future
    • Also fail the future if JS loading fails.

I've tried to run the test with:

flutter test --platform chrome

And you're right, the JS fails to load, and then everything else times out. I'm not sure why.

The test can be run manually with:

flutter run -d web-server test/wakelock_plus_web_plugin_test.dart

And see how some tests pass, but calling disable() after enable() seems to be broken in the underlying JS implementation.

@ditman ditman closed this Mar 5, 2024
@diegotori
Copy link
Collaborator

@ditman I'll take a look at this sometime this week.

@ditman
Copy link
Author

ditman commented Mar 6, 2024

@diegotori no problem, I was just cleaning up notifications and pending work on my end! I closed this because this was more of an "here are some ideas" type of PR, rather than anything that actually thought should be merged as is!

@diegotori
Copy link
Collaborator

@ditman sorry to dig up this one from the grave, but it looks like I was able to make the tests pass by waiting a few milliseconds after enabling the wakelock on web. In other words, in addition to fixing the way it loads the script using ui_web's AssetManager, it turns out that the no_sleep.js script doesn't await for the wakelock to be enabled in an asynchronous manner.

Right now, it waits for the result when calling enabled on it, since that JS function is async. However, the other enable/disable setter functions are not. Since my JS skills aren't as adept, I await someone to modify the script so that it sends back promises when calling enable/disable on the JS functions.

You can track the progress in #75.

@ditman
Copy link
Author

ditman commented Jan 8, 2025

@diegotori thanks for picking this up, I had totally forgotten about this PR, it's as if somebody else had written it :P

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.

2 participants