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

add basic web support #2014

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Conversation

b4s36t4
Copy link

@b4s36t4 b4s36t4 commented Mar 14, 2025

Basic web support for omi.

/claim #2008

Working flow

  • Sign-in
  • Onboarding
  • Twitter Flow
Kapture.2025-03-14.at.07.01.14.mp4

@kodjima33
Copy link
Collaborator

kodjima33 commented Mar 14, 2025

will check asap

cc @neooriginal @mdmohsin7 0-10 rating pls

@mdmohsin7
Copy link
Member

mdmohsin7 commented Mar 15, 2025

will check asap

cc @neooriginal @mdmohsin7 0-10 rating pls

Honestly I'd rate it 5/10, there are better approaches than this. Although they (better approaches) require a bit more work, but they guarantee better code maintainability

@kodjima33
Copy link
Collaborator

@b4s36t4 sorry bro 5/10 is too bad

@kodjima33
Copy link
Collaborator

@b4s36t4 btw I tried "flutter run -d chrome" and still got old flow

@b4s36t4
Copy link
Author

b4s36t4 commented Mar 16, 2025

Hi, @kodjima33. I don't agree on rating because there's not much of changes I have done and it's pretty much firebase configuration I have done. anyhow I have made some cleanup to streamline the process.

Before proceeding these are the following questions I have.

  • Should web follow ios principles or android principles? As an example Platform.isIOS will fail in web but components still can run on web, in this case should we use ios or android value? this should same across the app, there are multiple places we follow this condition.
  • Currently websocket.connect internally use http upgrade to connect with websocket with headers support but the client they're using is HTTPClient which is relying on Platform and raising errors. To support this we should allow websocket authentication over other methods (maybe query-param? or Sec-WebSocket-Protocol header). What should we follow?
  • Allowing cors? since browser relies heavily on secure cors headers, we should do it. Should we make them configurable from env or default to localhost (since cors only ever be required on web?)

Let me know what you think?

@b4s36t4
Copy link
Author

b4s36t4 commented Mar 16, 2025

@b4s36t4 btw I tried "flutter run -d chrome" and still got old flow

Means? just blank screen or any other issue? I have pushed some changes, can you try now?

@@ -86,7 +86,7 @@ dependencies:
geolocator:
version: ^3.0.2
webview_flutter: ^4.8.0
flutter_sound: ^9.10.0
flutter_sound: 9.10.0
Copy link
Author

@b4s36t4 b4s36t4 Mar 16, 2025

Choose a reason for hiding this comment

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

This is required, because latest version is causing issues with latest version (https://github.com/Canardoux/flutter_sound_web/blame/b64335b030592f8b36c4459ce2803b3ad29a0ebb/lib/flutter_sound_media_player_web.dart#L83-L86) which works on latest version of web dependency, but we're using file_picker which uses 0.5.1 version. I don't want update file_picker not sure what breaking changes they have. So I had to pin the version.

@b4s36t4
Copy link
Author

b4s36t4 commented Mar 16, 2025

hey, @kodjima33 let me know when you take a look at this questions. I solved 2nd question issue from the other PR i.e mac support. Regardless of the PR approval that's a small change, can be accommodated easily.
thanks!

@beastoin
Copy link
Collaborator

man, sry but i am not a fan of the fragmented control flow code (if-else), even in the current omi repo. you could do it better with a systematic way to handle platform-specific code.

looking forward.

@b4s36t4
Copy link
Author

b4s36t4 commented Mar 17, 2025

Hey, @beastoin can you answer above questions please. That would give me more scope to cleanup. Also I don't think there are any new if/else condition that can be extended to other way of multi-platform code. I just don't want to duplicate the whole code and pollute with same repeating stuff.

Please answer the above questions that would surely help me clean and remove a lot of if/else.

@beastoin
Copy link
Collaborator

Should web follow ios principles or android principles?

What do you mean about the principles? Tell me more about what Flutter supports for web and what do you suggest? For me, web is web, neither Android nor iOS.

Currently websocket.connect internally use http upgrade to connect with websocket with headers support but the client they're using is HTTPClient which is relying on Platform and raising errors.

Tell me more about the errors if you need my help. Since, with the current Omi backend system, you could use a simple command line to connect to WS. E.g.

wscat -c "wss://omi.local/v1/trigger/listen?language=zh&sample_rate=16000&codec=opus&uid=REDACTED" -H 'Authorization: REDACTED'

Allowing cors?

Of course, CORS is a must.

@b4s36t4 if I were you, I would:

  1. Check all the current platform-specific code, then wrap them to functions. With every function, you could use tons of if-else if you want to specify which code (logic, component) for which platform.
  2. Check the current libs which the project depends on, which ones do not support Web and any risk from that?
  3. Wrap all new platform-specific code (e.g., BLE connection) to functions, then use the same strategy as in 1.

Make it work and keep it simple like that first, then you could add more design patterns to make it cleaner later (if needed).

Join us https://discord.omi.me and feel free to dm me @thinh

@beastoin beastoin marked this pull request as draft March 24, 2025 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants