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

client1 + client2: Fix onsubmit event handler to allow submitting forms #462

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

SmithChart
Copy link
Contributor

According to the documentation
(https://lona-web.org/1.x/api-reference/views.html#post ) it should be possible to submit a <form> using POST to an interactive lona view. Without this change this fails with a
Uncaught ReferenceError: lona_window is not defined in the browser.

This change fixes the missing reference and allows to use the POST method to submit data to the view.

@fscherf
Copy link
Member

fscherf commented Aug 7, 2023

@SmithChart: Nice! Can you add a test too?

@codecov-commenter
Copy link

codecov-commenter commented Aug 7, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.10% 🎉

Comparison is base (7f8ba4c) 71.81% compared to head (a684c7b) 71.92%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #462      +/-   ##
==========================================
+ Coverage   71.81%   71.92%   +0.10%     
==========================================
  Files          84       84              
  Lines        5745     5745              
  Branches     1235     1235              
==========================================
+ Hits         4126     4132       +6     
+ Misses       1339     1336       -3     
+ Partials      280      277       -3     

see 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SmithChart
Copy link
Contributor Author

Since tests rely on Docker I am not sure if I can actually run the tests anywhere. I'll look into that.

@SmithChart SmithChart force-pushed the master branch 3 times, most recently from eb33388 to a684c7b Compare August 9, 2023 06:46
@SmithChart
Copy link
Contributor Author

@fscherf please have another look. I've added a test. But I am not sure if it should be put somewhere specific into the ordered block. From my point of this looks OK :-)

@fscherf
Copy link
Member

fscherf commented Aug 12, 2023

@SmithChart: Nice! I refactored your test a bit (https://github.com/lona-web-org/lona/tree/fscherf/fix-post-requests)
If you ok with the changes just squash my commit into yours. Also please remove the black configuration from the pyproject.toml.
After that the PR should be mergable

According to the documentation
(https://lona-web.org/1.x/api-reference/views.html#post ) it should be
possible to submit a `<form>` using `POST` to an interactive lona view.
Without this change this fails with a
`Uncaught ReferenceError: lona_window is not defined` in the browser.

This change fixes the missing reference and allows to use the `POST` method
to submit data to the view.

(Also add a test to make sure a POSTed `<form>` still works in the
future.)

Signed-off-by: Chris Fiege <cfi@pengutronix.de>
@SmithChart
Copy link
Contributor Author

@fscherf: I have squashed your suggested changes. Thanks for the update!

@fscherf
Copy link
Member

fscherf commented Sep 6, 2023

@SmithChart: Great! Thanks for contributing!

@fscherf fscherf merged commit 77b4091 into lona-web-org:master Sep 6, 2023
5 checks passed
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