Add option to warn user before leaving create or edit page

[jnoordsij]

Sometimes, especially on CRUD entries that take a long time to create or edit, it is useful to warn users if they attempt to leave the page by other means then the save button, to prevent accidental loss of input.

This PR adds a warnBeforeLeave config option that can be set both globally (in the config file) or per crud operation (with the setWarnBeforeLeave method).

[pxpm]

Wow, Wow Wow! I think this is NUTS!

Belive me or not, I'v dealt with the frustation of clicking on sidebar with an almost fill form!

I don't think this is a BC, so I will be reviewing this in the next days (maybe this week, but can't promise). Just letting you know that I liked and saw it :)

Best,
Pedro

[promatik]

Belive me or not, I'v dealt with the frustation of clicking on sidebar with an almost fill form!

I've been there too @pxpm, it's possible that everyone did 😅

@jnoordsij this is great 🙌 I think it's a must feature for backpack.
Backpack is on feature freeze, but I think we all agree this can sneak in in v4.1, let's hear @tabacitu about that 😅

I have only one concern, it should be triggering the alert only when user has changed anything on the form, so he is actually loosing changes when leaving. Right now it's triggering always ...

[jnoordsij]

Belive me or not, I'v dealt with the frustation of clicking on sidebar with an almost fill form!

I've been there too @pxpm, it's possible that everyone did 😅

@jnoordsij this is great 🙌 I think it's a must feature for backpack.
Backpack is on feature freeze, but I think we all agree this can sneak in in v4.1, let's hear @tabacitu about that 😅

Thanks! Would be great to have it merged in quickly; with the config option to trigger it it shouldn't change behavior on existing installations. You could still change the default config to true on the next major release.

I have only one concern, it should be triggering the alert only when user has changed anything on the form, so he is actually loosing changes when leaving. Right now it's triggering always ...

Yeah this seems to differ between browsers, but I already noticed this on my setup. I'm not sure how much of a problem it is (you don't really accidentaly land on these edit pages I guess, so it isn't that much of an annoyance to have an additional popup?). But you could try adding some global dirty JS variable that is set to true on an edit in any of the forms fields.

[tabacitu]

Great job here @jnoordsij . I agree with you guys, this is a cool feature to have inside the Create & Update operations (and maybe some other custom operations too).

We'll merge this in 4.2 - currently we're in a feature freeze. Indeed it wouldn't be a breaking change to merge this in 4.1, but we have to draw the line somewhere - otherwise we'll never stop working on 4.1. Plus, I think it's a feature that people need to know about, and they can do that when upgrading to 4.2, otherwise it'll go unused.

I just have some ideas about naming, I like the implementation 👏👏👏

[scrutinizer-notifier]

The inspection completed: 3 new issues, 3 updated code elements

[jnoordsij]

Hey there! Just wondering if there is any ETA on the 4.2 release and possibly merging this, so I can decide wether to manually implement these changes (and maybe my other PR's) in our application in the meantime or just wait a bit longer for the actual release. Thought I found somewhere that the planned release was around november, but couldn't find any schedule for it now.

[pxpm]

@jnoordsij hey there man 👋

4.2 is a little out of schedule, some bits that we planned to go fast had been going a little bit slower, but I don't think it would slip by much, we still working on it everyday.

@tabacitu created a "Feature Pool" https://github.com/Laravel-Backpack/CRUD/discussions/3960, so while we work on core fixes we would also work on some most desired features by our users to "compensate" from this little delay. I think this one just didn't have enough tracktion to be on list.

I still think this is a very nice feature, I don't think it's one of the most desired ones, but I think it's a cool one, easy to understand and implement.

@tabacitu is already aware of this by the name pings, if he decides we should do this from start in 4.2 I've nothing against.

I say: - if we don't find some incompatibility here that would take us some time to come-up with a solution/fix, all the heavy work is done by @jnoordsij , I would merge this @tabacitu !

Let me know,
Pedro

[tabacitu]

Thanks for bumping this @jnoordsij ! I still like this. A LOT. Both the idea and the implementation. I think we should even consider making it true by default in 4.2 - what do you guys think?

Also, about the WarnBeforeLeaving trait... I think @promatik 's spidey sense might have been right, this trait isn't really needed, feels like it pollutes the CrudPanel namespace a bit. Sure it's easier to do CRUD::setWarnBeforeLeaving(true), but not MUCH easier than doing CRUD::set('warnBeforeLeaving', true). Though one gets autocompletion, the other doesn't.

Either way, I've removed this trait, so the docs should say CRUD::set('warnBeforeLeaving', true). Still wondering whether to make this true by default in 4.2 - looking forward to hearing what you guys think.

Thanks again for this contribution @jnoordsij , we really appreciate it 🙏
Merged 🎉🎉🎉

[jnoordsij]

Great, thanks for merging this! Looking forward to the new version 😄

[tabacitu]

No, thank YOU. What do you think - should this default to true for all CRUDs? 👀

[jnoordsij]

I think defaulting it to true is a great idea, as probably most people will be really happy to have this. Moreover you can mention it in the upgrade guide as it is now moved into the new version and people can easily opt-out if they want to.

[pxpm]

I am not sure this should default to true.

It's a great feature but also a very intrusive one, if your real desire is to leave the page without sending the form, you will be greeted by the warning, I am not sure but I think this is usefull in some scenarios, but there is also scenarios where this really is not that necessary. (thinking of very small forms and such).

I would use this functionality in big cruds, but I think I am not going to activate this in small simple cruds where a mistake is not that costly, because, like I said, it happened to me leaving the form when I didn't wanted to, but also leave the form because I wanted to.

So, my opinion is that we should let the behaviour to be enabled by developer if/when needed.

Pedro

[leexuyiu]

hi there, im new to backpack and i was trying out this feature from the doc here

https://backpackforlaravel.com/docs/5.x/release-notes#warning-before-leaving-the-create-or-update-form

however when i tried CRUD::set('warnBeforeLeaving', true) in the setup() method, it doesnt work.

just to add, CRUD::setOperationSetting('warnBeforeLeaving', true) on specific operations are working.

can anyone confirm this? or maybe its only on my side?