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

refactor, fix and add feat to js bridge #132

Merged
merged 49 commits into from
Jul 13, 2023

Conversation

JOTSR
Copy link
Contributor

@JOTSR JOTSR commented Jul 12, 2023

Summary

Encapse private methods and attributes inside a class to securise access with private fields. Support JSON handling. Fix logic and types errors. Simplify code...

Motivation

One of the challenges of this project is the JS bridge, which is difficult to maintain for contributors and hard to discover for new users. A possible improvement is to rewrite the bridge using typescript, which has several benefits. First, it improves discoverability, as typescript can help with code completion and documentation. Second, it improves interoperability by providing type hints and safeguards. Third, as typescript is used in source code it don't affect the final execution (only JS is embedded). Moreover, for internal use, typescript can prevent errors (many references and typos in the original JS bridge have been caught by typescript) and improve project structure by enforcing types and modules.

Main changes

  • Add JSDoc to public API
  • Remove duplicated code and definitions
  • Remove deprecated code and browser API
  • Polyfill unstable and no generalized navigation API used to handle navigation
  • Fix types issue and declarations and prevent future bugs
  • Add new feature to rebind webui listeners when user inject new html
  • Rename public API for better clarity
  • Replace dangerous eval
  • Add missing declaration
  • Replace deprecated use of "var"
  • Update code to common modern js usage

JOTSR added 30 commits July 10, 2023 16:31
Navigation API is experimental and not in firefox, safari of opera, need to polyfill
Keep internal attributes and methods private
Construct Uint8Array with "of" instead of push data into it. Use template string. Remove temp constants.
use Uint8Array.of to construct buffer instead of pushing datas through a loop
Reduce code nesting. Remove deduplicated code. Improve readability. Use template literals instead of string concatenation. Use Uint8Array.of instead of pushing manually data into. Simplify code.
Use addRefreshableEventListner instead of directly binding event listeners to automatically attanch listener if html code is injected client side by the user.
@AlbertShown
Copy link
Contributor

This is really a big improvement to the project. Thank you @JOTSR 👍
We should add instruction in the readme to explain that to build WebUI we need to install esbuild /xxd

@JOTSR
Copy link
Contributor Author

JOTSR commented Jul 12, 2023

Ok I will add it later.
I'm currently dealing on resolving issue #98 by adding add set/get_title, set/get_size, set_position (get_position is a future work as no common browser API allow this).

@petabyt
Copy link
Contributor

petabyt commented Jul 13, 2023

Ok I will add it later. I'm currently dealing on resolving issue #98 by adding add set/get_title, set/get_size, set_position (get_position is a future work as no common browser API allow this).

#133 should make that easier

@AlbertShown AlbertShown merged commit 979b0b5 into webui-dev:main Jul 13, 2023
3 checks passed
@AlbertShown
Copy link
Contributor

adding add set/get_title, set/get_size, set_position

I don't think we should add title to WebUI because the end-user will use HTML <title></title> in all scenarios.

The APIs get_title, get_position and get_size can be added as TypeScript functions in the WebUI-Bridge, then we make a new C core function to call those JavaScript functions. But I vote not to do it. The end-user can write his custom JavaScript function if needed. But feel free to add them if you like too 👍

@JOTSR
Copy link
Contributor Author

JOTSR commented Jul 14, 2023

@AlbertShown As title, position and size is a part of the "external" aspect of the UI, proposing an turnkey solution is less boilerplate to implement for the end user. API to manage the rendered UI are out of scope obviously.

example_feat.mp4

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.

3 participants