signalpoint / DrupalGap

An application development kit for Drupal websites.
https://www.drupalgap.org
GNU General Public License v2.0
232 stars 185 forks source link

How to inform user when leaving a page with a form without saving #1021

Open luxio opened 5 years ago

luxio commented 5 years ago

Is there a way, to discover, if the user is leaving a page with a form without saving it. He should receive a message, that he has not submitted the form and the data will be lost, if going to another page without saving the path first.

It could be done by changing drupalgap_goto(), but maybe there is another way.

signalpoint commented 5 years ago

@luxio I don't think it is currently possible, but it should be pretty easy to get into core. For example, dg7 core already has hook_drupalgap_goto_preprocess(): https://github.com/signalpoint/DrupalGap/blob/7.x-1.x/src/modules/api/api.js#L192

That could be used as your logic point for determining if there is a form on the page, and seeing if it is partially completed, and then throw up an alert.

I think if we return false from this hook, and then modify drupalgap_goto() to notice a false return value and then have it stop, would be a way to get this working.

The invoke is here: https://github.com/signalpoint/DrupalGap/blob/3630ec06f00e95d2dd3da7bd09456ae138ae1de1/src/includes/go.inc.js#L28

Your thoughts?

luxio commented 5 years ago

@signalpoint That is exactly how I want to solve it.

luxio commented 5 years ago

@signalpoint What do you think about this approach:

function drupalgap_goto(path) {
  // ...

  // Invoke all implementations of hook_drupalgap_goto_preprocess().
  let goto_preprocess_invocation_results = module_invoke_all('drupalgap_goto_preprocess', path, options);
  if (goto_preprocess_invocation_results) {
    for (var stop_navigation in goto_preprocess_invocation_results) {
      if (stop_navigation == false) {
        //stop the navigation attempt
        return false;
      }
    }
  }

  //...
}

function mymodule_drupalgap_goto_preprocess(path) {
  try {
    var current_path = drupalgap_path_get();
    var options = {};

    if (arguments[1]) {
      options = arguments[1];
      if (typeof options.leave_form === 'undefined') {
        options.leave_form = false;
      }
    }
    if ((current_path.startsWith('node/add') ||
      (new RegExp('node\/[0-9]+\/form')).test(current_path)) &&
      options.form_submission == false &&
      options.leave_form == false) {
      navigator.notification.confirm(
        t('Do you really want to leave this page without saving first? Unsaved data will be lost.'), // message
        function (buttonIndex) {
          if (buttonIndex == 1) {
            // leave form
            options.leave_form = true;
            drupalgap_goto(path, options);
          }
        },
        t('Unsaved Data'),          // title
        [t('Yes'), t('Cancel')]     // buttonLabels
      );
      return false;
    }
  }
  catch (error) {
    console.log('mymodule_drupalgap_goto_preprocess - ' + error);
  }
}
signalpoint commented 5 years ago

@luxio Your proposed changes for drupalgap_goto look good. My only concern is this line for all existing implementation of hook_drupalgap_goto_preprocess():

if (stop_navigation == false) {

Since all existing implementations out there would be returning nothing, would that "nothing" successfully evaluate to false here?

luxio commented 5 years ago

@signalpoint

Since all existing implementations out there would be returning nothing, would that "nothing" successfully evaluate to false here?

If all implementations return nothing, the goto_preprocess_invocation_results should be an empty array, so

for (var stop_navigation in goto_preprocess_invocation_results) {

should prevent calling

if (stop_navigation == false) {

Or am I missing something?

signalpoint commented 5 years ago

@luxio That sounds correct. If you wouldn't mind, please do a test run with an implementation of that hook that returns nothing, and just make sure it doesn't stop the navigation, thank you!

luxio commented 5 years ago

@signalpoint

If you wouldn't mind, please do a test run with an implementation of that hook that returns nothing, and just make sure it doesn't stop the navigation, thank you!

The implementation hook mymodule_drupalgap_goto_preprocess() above already returns nothing, if current_path does not match the conditions in the if clause.

It does not break the navigation in my app.

signalpoint commented 5 years ago

@luxio Excellent, thanks for checking. If you'd like to make a PR, we'll get your improvement merged in.