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

Warning users of unsaved changes #144

Closed
matbind opened this issue Oct 8, 2024 · 8 comments
Closed

Warning users of unsaved changes #144

matbind opened this issue Oct 8, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@matbind
Copy link

matbind commented Oct 8, 2024

Hi, thanks for this great package!

It would be really helpful to have built-in functionality for warning users when they attempt to navigate away from a page with unsaved changes in the CKEditor field, similar to the default behavior in Nova's standard fields (like Text).

If this feature is out of scope, I’d be happy to implement it using custom JavaScript. However, I’d need to access the CKEditor instance. Could you provide guidance on the intended (or easiest) way to access the CKEditor instance within Nova?
It seems one can get the instance through the HTML element but this is cumbersome.

Thanks in advance!

@mostafaznv mostafaznv added the enhancement New feature or request label Oct 9, 2024
mostafaznv added a commit that referenced this issue Oct 9, 2024
feat: alert before unsaved changes #144
@mostafaznv
Copy link
Owner

Hi @matbind,

Your requested feature has been implemented, and you can try it using the latest version.
Please update to 7.7.0 and let me know the result.

This feature is enabled by default; however, you can disable it using the configuration file or CKEditor’s methods.

For more information, please take a look at this and this.

'alert-before-unsaved-changes' => true,

@matbind
Copy link
Author

matbind commented Oct 9, 2024

Works like a charm! It still shows the alert even if the user reverts their changes, but this is definitely good enough. Thanks
for this amazingly quick response! You're the man!

@matbind
Copy link
Author

matbind commented Oct 9, 2024

Hi again @mostafaznv,

I just noticed that the alert only shows up when reloading the page, not when going back or navigating somewhere else within Nova. In this regard it is different from e.g. Text. Could you maybe extend the feature to also include these cases?

Thanks!

mostafaznv added a commit that referenced this issue Oct 9, 2024
fix: display alert both before reloading and when pressing the browser back button #144
@mostafaznv
Copy link
Owner

Hi again.

I can detect when a user reverts their changes, but I didn’t implement it for two reasons:

  1. Laravel Nova doesn’t detect it either.
  2. For large content, it could add unnecessary overhead.

As for the browser’s back button, it should be working now. Please update to the latest version (7.7.1).
I am now fully relying on Nova’s built-in features to prevent users from leaving the current page.

@matbind
Copy link
Author

matbind commented Oct 10, 2024

Hi, and thanks for another quick update!

You are absolutely right about reverted changes, Nova doesn't detect it either, my mistake! :)

I tried 7.7.1 and made the following observations:

  • Back button: The browser navigates back even when clicking cancel in the dialog
  • Clicking a link in Nova's sidebar: The browser shows the dialog multiple times when clicking cancel. Looks like it shows it twice if one editor field is present, and one more time for each additional editor field (a little extrapolation here on my part: I have two editor fields in my resource and the dialog is shown three times)
  • Page reload works like intended
  • Closing browser tab works like intended

I tested this in Chrome and Firefox.

Could you please take another look?

mostafaznv added a commit that referenced this issue Oct 12, 2024
fix: prevent multiple alert prompts #144
@mostafaznv
Copy link
Owner

Hi again,
Thank you for your detailed report and investigation. I really appreciate it.
Apologies for shipping a buggy feature. I’ve been quite busy lately and didn’t have much time to check it carefully.
I’ve now published another fix, and hopefully, this will be the last one :D

Please check it again, and let me know if the issue is resolved.

@matbind
Copy link
Author

matbind commented Oct 14, 2024

Hi!

It works great now, thank you for the quick responses. No worries about you being busy, you are doing all this for free which makes your response times all the more insane.
Glad that I was able to contribute to this great package, if only in ideas and QA haha.

Thanks again for being such a swell dude!

@mostafaznv
Copy link
Owner

You’re welcome, @matbind
It’s kind of you, and thank you for your heartwarming words.

I’m glad it helped you, and I appreciate your contribution 🤘

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants