(fix): web debug error - #622

[eggp]

Hello @jonbhanson,

Changes:

1. Added initWeb to FlutterNativeSplash.
2. Added the native JavaScript function showSplashWeb to template.dart and created a Dart function for it.
3. Added the native JavaScript function hideSplashWeb to template.dart and created a Dart function for it.
4. Refactored the JavaScript function removeSplashFromWeb.
5. Refactored the method FlutterNativeSplashWeb.handleMethodCall.

Why were these changes necessary?

In issue #607 (#607), I did not consider that it works with Flutter hot reload (at that time, there was a bug in the Android Studio Flutter plugin, and hot reload did not work). We cannot detect hot reload. However, it only occurs in debug mode, so by checking kDebugMode, we can determine if hot reload is running (assuming that it runs with hot reload in debug mode). In this case, we do not clean up the code from the HTML. We simply hide the overlay, and to prevent it from interfering at the end, we ensure this with display=none.

However, an additional step is needed in debug mode (side effect :( ). Therefore, initWeb() was introduced:
Usage:

WidgetsBinding widgetsBinding = WidgetsFlutterBinding.ensureInitialized();
FlutterNativeSplash.initWeb();

WidgetsFlutterBinding.ensureInitialized() must be called before invoking FlutterNativeSplash.initWeb().

(Note: initWeb checks whether the Dart code is running on the web, so in the case of hybrid apps, the app using the library does not need to handle it separately.)

[abdelaziz-mahdy]

just an idea to avoid adding the function into main cant we update the loading on web?

<script>
  window.addEventListener('load', function (ev) {
    _flutter.loader.loadEntrypoint({
      serviceWorker: {
        serviceWorkerVersion: serviceWorkerVersion,
      },
      onEntrypointLoaded: async function (engineInitializer) {
        let appRunner = await engineInitializer.initializeEngine();
        await appRunner.runApp();
        // i mean we call remove splash screen here
      }
    });
  });
</script>

this will result in no need to add the function to main app.

[eggp]

@zezo357

Good idea, and in the end, you are right.

But personally, my only problem with it is that currently, the Flutter plugin Dart code is the 'conductor,' and in JavaScript, only the commands are present, which the conductor controls. If we move this responsibility to JavaScript, it becomes unclear whether the control is happening from the Dart or JS side.

Furthermore, if you look at my code, the overlay appears by default, and nothing happens with the first showSplashWeb call because the if statement prevents it. This whole technique is really only necessary for debugging, as there is no hot reload in release mode. Even the kDebugMode check could be placed in the initWeb method, and it would still work:

static void initWeb() {
  if (kIsWeb && kDebugMode) {...

Or, if you don't call the initWeb() function, there is no error; it's just that the splash screen won't appear in the case of a hot reload.

All in all, I'm not sure if I would include what you suggested in the Flutter JS init because it appears unnecessarily in release mode and, in my opinion, disrupts the control flow.

[abdelaziz-mahdy]

@eggp

Thank you for explaining, I didn't take into consideration the mentioned points, I just wanted to maintain the previous behavior to not break other apps using older version of the package,

I know it's just a one function call, but this function call if missing breaks the app, since the splash never gets removed(correct me if I am wrong)

So using the above method we would have maintained the old behavior, and that didn't break anything from my point of view,

If the remove function can be called more than once then all is good, having it called from js or dart is not a problem.

If that's not possible or it will break some other things, should the 2.3.6 be reverted and replaced with 3.0.0 as it's a breaking change?

[eggp]

@zezo357

Hello,

I've been thinking a lot about whether this is a breaking change. Technically, yes, but not necessarily in practice.
Why?

There is no breakage in production (release) mode.
In debug mode, there is a break, but there is no error. The splash screen will not appear in the case of hot reload unless initWeb() is called.

[abdelaziz-mahdy]

@eggp check my issue again, the main problem was not adding the remove leaded to the splash not being removed, but in 2.3.5 it gets removed,

2.3.6 remove has to be added to main

2.3.5 no remove needed(package is a dev dependency)

And in 2.3.6 having the remove resulted in the problem that this pr fixed.

So the breaking change was that the remove splash needs to be added or init web

[eggp]

@zezo357

Hello, please look at what I wrote. The error you indicated was caused by a modification I proposed, so I corrected it within the scope of the task and submitted a pull request.

Since this database belongs to @jonbhanson, I would leave the final decision to him.

[abdelaziz-mahdy]

@eggp yes i understand the fix and it's very good, thank you for it

But I am talking about the splash not being automatically removed,

The remove led to the error I mentioned, which you have fixed in this pr, and thanks for that

But the breaking change I am mentioning is the need to add initweb or remove to main which in 2.3.5 it's not needed

Anyway I will test again on 2.3.6 even though I am sure it doesn't get removed without calling the remove function

[abdelaziz-mahdy]

Also I think you are correct about dart being the caller of js , so just changing the version while adding the docs needed is enough that's my own opinion.

[eggp]

@zezo357

Hello,

Roll back to version 2.3.4, and even though the splash seems to disappear without calling the remove, in reality, if you inspect the DOM, everything stays there, and only the Flutter element is placed over the splash element by CSS. Additionally, if you put a console.log in the JavaScript code, you can see that the function is not executed. So, what you're saying is actually a faulty operation.

For it to disappear automatically on the web without calling remove, it would need to be captured by some Flutter event, but I haven't found a suitable one for this yet :(

[abdelaziz-mahdy]

I did rollback, I am just stating the problem so it can be fixed without causing someone a problem in production without knowing why

[abdelaziz-mahdy]

@zezo357

Hello,

Roll back to version 2.3.4, and even though the splash seems to disappear without calling the remove, in reality, if you inspect the DOM, everything stays there, and only the Flutter element is placed over the splash element by CSS. Additionally, if you put a console.log in the JavaScript code, you can see that the function is not executed. So, what you're saying is actually a faulty operation.

For it to disappear automatically on the web without calling remove, it would need to be captured by some Flutter event, but I haven't found a suitable one for this yet :(

Yes I did notice it was not removed and just overlayed with the app, but now it doesn't 😅

[jonbhanson]

Thank you for this PR, @eggp. However, I think it will be best to introduce the web fade in a new major revision as I don't know if a way to add it without it being a breaking change.

[abdelaziz-mahdy]

just an idea to avoid adding the function into main cant we update the loading on web?

<script>
  window.addEventListener('load', function (ev) {
    _flutter.loader.loadEntrypoint({
      serviceWorker: {
        serviceWorkerVersion: serviceWorkerVersion,
      },
      onEntrypointLoaded: async function (engineInitializer) {
        let appRunner = await engineInitializer.initializeEngine();
        await appRunner.runApp();
        // i mean we call remove splash screen here
      }
    });
  });
</script>

this will result in no need to add the function to main app.

@jonbhanson can this help?

[jonbhanson]

@zezo357 one of the features of this package is the ability to use FlutterNativeSplash.preserve and FlutterNativeSplash.remove() to keep the splash on screen while initialization work is done and then remove the splash screen programmatically within the Flutter app. I think if we used your code as suggested, it would remove this ability to keep the splash on screen and remove it programmatically. Am I overlooking something?

[abdelaziz-mahdy]

Yes it would do that, but right now the preserve was not working? I didn't try it before but the splash was hidden behind the app so it was not working?

Let me know if I am missing something

[jonbhanson]

I am not aware of any problem with .preserve and .remove in the current version of the package. I tested the example app just now and it seems to work fine.

[abdelaziz-mahdy]

I am not aware of any problem with .preserve and .remove in the current version of the package. I tested the example app just now and it seems to work fine.

Yes, I just tested it and it's working. I initially thought the Flutter engine would overlay it, but after reviewing your code, I realized you've designed it to stop rendering until the 'remove' is called. That's a very nice solution!

and yes for my solution will disable the preserve so i think its not viable workaround, so a breaking change version is the way to go