search

openfoodfoundation / openfoodnetwork

Connect suppliers, distributors and consumers to trade local produce.
https://www.openfoodnetwork.org
GNU Affero General Public License v3.0
1.12k stars 724 forks source link

[Admin][Order cycles] Always show warning when leaving page with unsaved changes #10456

Closed drummer83 closed 1 year ago

drummer83 commented 1 year ago

Description

This was found when testing #10351. When creating or editing an complex order cycle (enterprise setting 'sells = any'), there is a warning popup when leaving the page with unsaved changes. However this popup is not shown, if there were previous changes saved and then more changes have been added (see screen capture below). This applies to all 4 steps of order cycle creation process.

Expected Behavior

The warning popup should always be displayed when leaving the page with unsaved changes.

Actual Behaviour

If changes are made, saved, then more changes made and then the page is left, there is no warning. Unsaved data will be lost.

Steps to Reproduce

  1. Go to /admin/order_cycles/xxIDxx/edit (or /incoming, /outgoing or /checkout_options).
  2. Make changes.
  3. Click save.
  4. Make more changes (without saving!).
  5. Leave the page (e.g. click on one of the other steps or any other menu item on the page).
  6. Observe that no warning is shown.
  7. Return to the page where you made the changes.
  8. Observe that the last changes are lost.

Animated Gif/Screenshot

https://user-images.githubusercontent.com/1790176/219463277-0d752ad4-52a3-4e4d-9a66-e23954cca60b.mp4

This is the popup which should be shown (Firefox): image

Workaround

Severity

bug-s4: it's annoying, but you can use it

Your Environment

Possible Fix

vviekk commented 1 year ago

Hi @drummer83

I tried to reproduce the issue on my local and made following observations:

  1. For step 1(/edit), and 2(/incoming) alert is shown for the first unsaved changes and not for the unsaved changes made after first save - (this issue)
  2. For step 3(/outgoing), alert is show for all the unsaved changes every time it was required. - working as expected.
  3. For step 4(/checkout_options)
    1. Save button is active even before any changes are made.
    2. Made changes(Unchecked a couple of boxes), and clicked to move to another step without saving. No alert for unsaved changes - (another issue?)

Please let me know if I'm missing something. :)

drummer83 commented 1 year ago

Hi @vviekk,

Thanks for following up on this! Here are my observations.

  1. For step 1(/edit), and 2(/incoming) alert is shown for the first unsaved changes and not for the unsaved changes made after first save - (this issue)

Yes, I confirm! :+1:

  1. For step 3(/outgoing), alert is show for all the unsaved changes every time it was required. - working as expected.

Yes, you are totally right. This one works as expected! :+1:

  1. For step 4(/checkout_options)

    i. Save button is active even before any changes are made.

Yes, this is intentional to provide an option to the user to always finish the setup of the order cycle (see here). :+1:

ii. Made changes(Unchecked a couple of boxes), and clicked to move to another step without saving. No alert for unsaved changes - (another issue?)

I can't reproduce this. Actually I always see the alert. So it seems to work as expected. Can you try again and maybe capture a screen video?

drummer83 commented 1 year ago

Ah, and please see here to cover the 'Cancel' button as well! I have updated the 'Steps to reproduce' above.

vviekk commented 1 year ago

@drummer83

ii. Made changes(Unchecked a couple of boxes), and clicked to move to another step without saving. No alert for unsaved changes - (another issue?)

I can't reproduce this. Actually I always see the alert. So it seems to work as expected. Can you try again and maybe capture a screen video?

Hm, this is what I see for step 4 (Right after landing on the page - 1st unsaved changes):

https://user-images.githubusercontent.com/91144693/222059834-6abedbe6-f3ed-426f-b83f-bcfc604f5967.mp4

please see https://github.com/openfoodfoundation/openfoodnetwork/pull/10351#issuecomment-1433235381 to cover the 'Cancel' button as well

For steps 1 and 2:

For step 3:

For step 4:

At the time of this comment, I have the latest code available on my local.

drummer83 commented 1 year ago

Okay @vviekk, that's really strange. What I don't understand is that on step 3 you have no distributor selected, but still you can see the checkout options of 'Mary's Online Shop'. This shouldn't happen - if no distributor is selected, then no checkout options should be shown.

Can you try again and select a distributor on step 3 first?

I agree with your way of adding the 'cancel' button to this issue.

If you like, you can start fixing step 1 and 2 first and leave step 4 out of scope until we find out what is happening there.

vviekk commented 1 year ago

@drummer83 Apologies about the confusion. There are 2 distributors listed under step 3. It just takes a while to populate. I went back to step 4 before they could load.

Thank you. I have started looking into the fix.

vviekk commented 1 year ago

@drummer83

For step 4, I now see the alert message for all the unsaved changes while leaving the page and for cancel button as well.

But seems like it doesn't save the changes made when Iclick save.

https://user-images.githubusercontent.com/91144693/222750330-d643be3d-55f2-4fec-a230-4ee6df4530d0.mp4

Could you please check if you can reproduce it or if its only my dev env?? Or if its an issue for only my dev env, I'll clean and reconfigure it.

drummer83 commented 1 year ago

@vviekk The changes are not saved in your case, because you must have at least one shipping and payment method available for each distributor. This is not the greatest usability and I am about to create issues to improve this, but it is out of scope here. EDIT: The issue is already there and we're working on a fix. https://github.com/openfoodfoundation/openfoodnetwork/issues/10441

If there is no technical need for the reload of the page, then we can remove that. But I am not a developer myself so we would need to ask @openfoodfoundation/core-devs for an opinion here.

mkllnk commented 1 year ago

If there is no technical need for the reload of the page, then we can remove that.

I'm not sure either but there was a case where a reload was the easiest solution because we couldn't tell AngularJS that the form had changed after changing inputs managed with StimulusJS. It might be possible though. You can try.

vviekk commented 1 year ago

Made the change to fix the alert for unsaved changes when clicking on cancel or leaving the page with unsaved changes for /edit and /incoming steps.

/outgoing and /checkout_options seems to be working as expected.