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

[Web] Expand "Interacting with Javascript" #8986

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

Faless
Copy link
Contributor

@Faless Faless commented Feb 17, 2024

Document the new (preferred) interface.

Add a small section about downloading files to the user device.

Based on: https://godotengine.org/article/godot-web-progress-report-9/

Should be back-ported to 3.x/3.5 for which it needs changes to the code block about to_utf8_buffer/to_utf8, create_callback argument types, and FileAccess/File .

@Faless Faless added area:manual Issues and PRs related to the Manual/Tutorials section of the documentation topic:export labels Feb 17, 2024
@Faless
Copy link
Contributor Author

Faless commented Feb 21, 2024

@TheYellowArchitect thanks for the review! Fixed :)

tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
@Faless
Copy link
Contributor Author

Faless commented Feb 21, 2024

@AThousandShips thanks! Fixed :)

Copy link
Member

@mhilbrunner mhilbrunner left a comment

Choose a reason for hiding this comment

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

Some suggestions below. In general, this tutorial uses a lot of code comments for explanations, which is problematic as they (currently) can't be translated, which admittedly is a fault of our tooling. Where possible, it would be good to move more explanations to the tutorial text.

Otherwise, this looks good to me and covers something frequently asked about :)

tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
tutorials/platform/web/javascript_bridge.rst Outdated Show resolved Hide resolved
Document the new (preferred) interface.

Add a small section about downloading files to the user device.
@Faless
Copy link
Contributor Author

Faless commented Feb 21, 2024

Some suggestions below. In general, this tutorial uses a lot of code comments for explanations, which is problematic as they (currently) can't be translated, which admittedly is a fault of our tooling. Where possible, it would be good to move more explanations to the tutorial text.

I've applied the suggestions.

As discussed in chat, I'd rather merge this and stop telling people to use eval (which is terrible) 3 years after this feature was merged.

Hopefully we can find someone to improve this in follow up PRs, since I won't have time to work more on this (I also have to port the Multiplayer Replication article).

EDIT: Also, thanks for the in-depth review! :)

@mhilbrunner mhilbrunner merged commit e3e0db3 into godotengine:master Feb 21, 2024
1 check passed
@mhilbrunner
Copy link
Member

Thank you! :) Merged.

@Faless Faless deleted the web/javascript_bridge branch February 21, 2024 11:22
@mhilbrunner
Copy link
Member

Cherry-picked to 4.2 in #9647.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation topic:export
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants