Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Kulikowski
dismissed
their stale review
Still investigating issue with form submit
beaufortfrancois
left a comment
beaufortfrancois
left a comment
There was a problem hiding this comment.
Thanks for adding cross-document support Pascal.
I've only reviewed demos/ and evals-cli/examples/french-bistro for now.
| validateForm(); | ||
|
|
||
| if (formValidationErrors.length && e.respondWith) { | ||
| e.respondWith(formValidationErrors); |
There was a problem hiding this comment.
respondWith doesn't exist in toolactivated event. I think it should be removed unless you use this somehow.
| } | ||
|
|
||
| if (isCrossDocument) { | ||
| // Trigger native form submission to navigate to result.html |
There was a problem hiding this comment.
cross document should not mean automatic submission. The user still needs to review what is about to be submitted and click the button.
There was a problem hiding this comment.
I think we can remove this entire block if/else
|
|
||
| ### Variation | ||
|
|
||
| When visiting `index.html?crossdocument`, the form submission triggers a navigation to `result.html`. This can be used to demonstrate cross-document tool execution. |
There was a problem hiding this comment.
Current code suggests you need ?crossdocument=1. If you want this, I'd suggest using params.has('crossdocument') instead of params.get('crossdocument') !== null
| return; | ||
| } | ||
|
|
||
| if (isCrossDocument) { |
There was a problem hiding this comment.
form.requestSubmit(); should be called there.
| e.preventDefault(); | ||
| form?.addEventListener('submit', function (e) { | ||
| if (!isCrossDocument) { | ||
| e.preventDefault(); |
There was a problem hiding this comment.
Let's always call preventDefault() there and remove the one after line 35
| }); | ||
|
|
||
| // On result.html, fill the modal with the reservation details. | ||
| if (location.pathname.includes('/result.html')) { |
There was a problem hiding this comment.
How about moving this block in results.html to not pollute script.js?
| @@ -0,0 +1,3 @@ | |||
| This directory contains evaluation test cases for the [WebMCP french-bistro!](../../../demos/french-bistro/) demo. | |||
There was a problem hiding this comment.
Should we expand a bit on how we can test both versions of this demo (same-document and cross-document) and results should hopefully be the same?
3 participants
Fixes #48
Extended the french-bistro site to optionally perform a navigation for the form submission.