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.

Sign up for GitHub

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

Jump to bottom

fix: replace flutter_keyboard_visibility with flutter_keyboard_visibility_temp_fork to support Flutter/Wasm target #2293

Merged
merged 6 commits into from
Sep 27, 2024
Merged

fix: replace flutter_keyboard_visibility with flutter_keyboard_visibility_temp_fork to support Flutter/Wasm target #2293

merged 6 commits into from
Sep 27, 2024

Conversation

EchoEllet
Copy link
Collaborator

Description

Replace flutter_keyboard_visibility with flutter_keyboard_visibility_temp_fork to support Flutter/Wasm target.

This is not a future-proof solution and we should replace the plugin, we might have our own solution in quill_native_bridge or develop a separate plugin.

Related Issues

Type of Change

  • ✨ New feature: Adds new functionality without breaking existing features.
  • 🛠️ Bug fix: Resolves an issue without altering current behavior.
  • 🧹 Code refactor: Code restructuring that does not affect behavior.
  • ❌ Breaking change: Alters existing functionality and requires updates.
  • 🧪 Tests: Adds new tests or modifies existing tests.
  • 📝 Documentation: Updates or additions to documentation.
  • 🗑️ Chore: Routine tasks, or maintenance.
  • ✅ Build configuration change: Changes to build or deploy processes.

@EchoEllet
fix: replace flutter_keyboard_visibility with flutter_keyboard_visibi…
7de4b33 
…lity_temp_fork to support wasm and be able to target latest version of compileSdkVersion
@EchoEllet
Copy link
Collaborator Author

EchoEllet commented Sep 27, 2024 •
edited
Loading

Was able to build the app for Flutter/WASM, however still having an issue running the example app:

Details

image

Which seems to be related to hydrated_bloc. See bloc #4230

After removing hydrated_bloc:

image

Seems like a simple issue that require minor changes.

Update:

image

Was able to fix those minor issues and seems to be running without any noticeable issues with Flutter/Wasm target on the web.

@EchoEllet
Copy link
Collaborator Author

EchoEllet commented Sep 27, 2024 •
edited
Loading

It looks like kIsWeb is true on Flutter/Wasm, and this conditional import uses web/quill_controller_web_stub.dart:

import 'web/quill_controller_web_stub.dart'
    if (dart.library.html) 'web/quill_controller_web_real.dart';

So our code expects to use quill_controller_web_real.dart due to the kIsWeb check when the conditional import is invalid for Flutter/WASM which uses quill_controller_web_stub.dart that throws exception.

The alternative of dart.library.html is dart.library.js_interop. Since we don't need quill_controller_web_real.dart unless #2220 (see 1998) is fixed, commented the code and removed it completely from being used, left a TODO with reference so other developers know why this is still not removed, I will address as soon as I can. We might also consider moving this functionality to quill_native_bridge_web.

This comment will be updated soon.

AtlasAutocode

This comment was marked as resolved.

Sign in to view

@EchoEllet

This comment was marked as resolved.

Sign in to view
@EchoEllet EchoEllet marked this pull request as ready for review September 27, 2024 18:42
@EchoEllet EchoEllet merged commit 18bb358 into master Sep 27, 2024
2 checks passed
@EchoEllet EchoEllet deleted the fix/wasm-build-failure branch September 27, 2024 18:44
@fdwl

This comment was marked as resolved.

Sign in to view
@radvansky-tomas

This comment was marked as resolved.

Sign in to view
@effmuhammad

This comment was marked as resolved.

Sign in to view
CatHood0 pushed a commit to CatHood0/flutter-quill that referenced this pull request Oct 28, 2024
@EchoEllet
fix: replace flutter_keyboard_visibility with flutter_keyboard_visibi…
da1ed15 
…lity_temp_fork to support Flutter/Wasm target (singerdmx#2293)

* fix: replace flutter_keyboard_visibility with flutter_keyboard_visibility_temp_fork to support wasm and be able to target the latest version of compileSdkVersion on Android

* fix(example): remove hydrated_bloc to run Flutter/Wasm

* fix(wasm): avoid using quill_controller_web_stub.dart for web which will throw exception due to usage of dart.library.html in a conditional check

* chore: remove web package as no longer used

* chore: add related comments to quill_controller_web_real.dart and quill_controller_web_stub.dart

* chore: format quill_controller_web_real.dart and quill_controller_web_stub.dart
@EchoEllet

This comment was marked as resolved.

Sign in to view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AtlasAutocode AtlasAutocode AtlasAutocode left review comments

@singerdmx singerdmx singerdmx approved these changes

@CatHood0 CatHood0 Awaiting requested review from CatHood0

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@EchoEllet @fdwl @radvansky-tomas @effmuhammad @singerdmx @AtlasAutocode