Skip to content

(feat): Enable external form Submission #550

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Twiineenock
Copy link
Contributor

@Twiineenock Twiineenock commented Jun 3, 2025

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

This PR introduces two custom React hooks — useExternalSubmitListener and usePostSubmissionCallback — into the FormEngine to improve its interoperability with external shells like the Form Display Engine (FDE).

🚀 What’s New

1. useExternalSubmitListener

Enables external triggering of form submission by listening for a global custom event (triger-form-engine-submit). This is especially useful when the FormEngine is embedded in another module's UI like the FDE, and submission needs to be controlled from outside (e.g., a shell toolbar or modal footer).

  • Registers a window event listener on mount.
  • Invokes the provided submitFn when the event is dispatched.
  • Cleans up the listener on unmount.

2. usePostSubmissionCallback

Supports deferring follow-up actions until after form submission completes. This allows external code to safely run logic (e.g., closing modals, navigation, showing notifications) after the form has finished submitting.

  • Listens for a form-submission-complete custom event.
  • Executes an action immediately if the form has already submitted, or defers it until setIsFormSubmitted(true) is called.
  • Prevents duplicate executions.

🧩 Use Case: Form Display Engine (FDE)

These hooks make it possible to:

  • Trigger form submission from an external shell.
  • Respond to submission completion in a decoupled, reliable way.

This integration improves reusability of the FormEngine across platforms while preserving single-responsibility and encapsulation within the component.

🧪 How they are used

  1. Use useExternalSubmitListener to listen for submission trigger events.

  2. Trigger a form submission by dispatching:

    window.dispatchEvent(new Event('triger-form-engine-submit'));
  3. Optionally, dispatch a post-submission callback:

    window.dispatchEvent(
      new CustomEvent('form-submission-complete', {
        detail: { action: () => console.log('Post-submit logic ran!') }
      })
    );

✅ Checklist

  • Custom hooks added with full documentation
  • Submission now triggerable externally
  • One-time post-submission callbacks supported
  • Backwards compatibility maintained
  • PR reviewed and tested locally

Screenshots

RFE-4-FDE

Screencasts

https://www.loom.com/share/193799987e0848a0b20e0575a462a3de?sid=c0d5143c-7aac-4e5f-bedd-aa3671833ffe

Related Issue

Other

Fast Data Entry: openmrs/openmrs-esm-fast-data-entry-app#115

Patient Chart: openmrs/openmrs-esm-patient-chart#2476

@samuelmale
Copy link
Member

samuelmale commented Jun 9, 2025

Thanks for looking into this @Twiineenock! However, I've got some reservations about using an event-based approach to trigger form submission. It would work well if O3 embraced the concept of singleton forms, but that’s not really the case. Dispatching a submission event means all mounted form instances would respond to it, and I’m not sure how that would play out with FDE’s "Group sessions."

Alternatively, we could use an imperative handle approach with forwardRef, or some kind of callback (I prefer the former). That way, we can also add an optional prop to control whether the cancel and submit buttons are included. (I'm happy to discuss further)

* })
* );
*/
export function usePostSubmissionCallback() {
Copy link
Member

Choose a reason for hiding this comment

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

Don't we achieve the same utility from onSubmit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @samuelmale ,
After further review, I also agree that we no longer need the usePostSubmissionCallback hook. The existing onSubmit handler sufficiently covers all post-submission scenarios at this point. Additionally, the useExternalSubmitListener hook (which we plan to rename) works seamlessly alongside onSubmit, and together they handle the flow effectively.

@Twiineenock
Copy link
Contributor Author

Thanks @samuelmale for the review. I am glad to provide a prototype of the imperative handle approach too!

Copy link
Member

@samuelmale samuelmale left a comment

Choose a reason for hiding this comment

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

@Twiineenock do we also need to support an event that triggers validation?

* // Somewhere else (e.g., external button or shell app)
* window.dispatchEvent(new Event('triger-form-engine-submit'));
*/
export function useExternalSubmitListener(submitFn: () => void, eventName = 'triger-form-engine-submit') {
Copy link
Member

Choose a reason for hiding this comment

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

If we are to make this hook more specific, I would change its signature slightly:

Suggested change
export function useExternalSubmitListener(submitFn: () => void, eventName = 'triger-form-engine-submit') {
export function useExternalSubmitListener(internalSubmitHandler: () => void) {

If the hook is defining a specific listener, I suggest defining the eventName locally and renaming it to something like "rfe-form-submit-action" or "rfe-trigger-form-submit".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Comment on lines 28 to 29
const handleSubmitWrapper = () => submitFn();
window.addEventListener(eventName, handleSubmitWrapper);
Copy link
Member

Choose a reason for hiding this comment

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

We should expect the event to provide more details about the target form session ie. provide the target form and patient UUID. This would help us map the dispatched event to the correct form instance:

const { formUuid, patientUuid } = useFormContext();
const handleSubmit = (event) => {
    const { formUuid: targetForm, patientUuid: targetPatient, encounterUuid /**while in edit mode **/} = event.details;
    if (targetForm === formUuid && targetPatient === patientUuid) {
         // invoke submission
         internalSubmitHandler();
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@@ -16,6 +16,7 @@ interface SidebarProps {
onCancel: () => void;
handleClose: () => void;
hideFormCollapseToggle: () => void;
isSubmissionTriggeredExternally?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

I would name this something like hideControls or hideInternalButtons which defaults to false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed to

@Twiineenock
Copy link
Contributor Author

Twiineenock commented Jun 18, 2025

@Twiineenock do we also need to support an event that triggers validation?

@samuelmale Good question!
So far, I haven't encountered a need for a separate validation trigger. The RFE automatically performs validation before it can submit a form.

@Twiineenock
Copy link
Contributor Author

I have updated the rest of the PRs within other repos to match these new changes

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.

2 participants