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

Make OC edit warning modal cancel button redirect user to OC list [OFN-12774] #12784

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

Conversation

wandji20
Copy link
Contributor

What? Why?

Make OC edit warning modal cancel button redirect the user to OC list

What should we test?

Verify that the cancel button in the edit OC warning modal links to the OC list.

  • Visit ... page.
    /admin/order_cycles/2/edit

Release notes

Order cycle warning modal cancel link redirects the user to Order cycle list

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes

Order cycle warning modal cancel link redirects the user to Order cycle list

Dependencies

N/A

Documentation updates

N/A

@rioug rioug added the user facing changes Thes pull requests affect the user experience label Aug 20, 2024
Copy link
Collaborator

@rioug rioug left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @wandji20 🙏

@filipefurtad0
Copy link
Contributor

This PR is conflicting with your other (just merged PR), it needs a rebase and conflict fix before testing. Thanks @wandji20 🙏

@drummer83
Copy link
Contributor

Hi @wandji20,
Thanks for fixing this last bit. 💪

I couldn't find any problems. Clicking cancel redirects to the order cycle list as expected now. There is an additional popup from the browser now. Is there anything we can do about that?

Kazam_screencast_00021.webm

Also Github is asking for another review. Do we need that or is it just due to the earlier conflict resolution?

Let me know and I will move this forward.
Thanks again!

@drummer83 drummer83 added feedback-needed and removed pr-staged-au staging.openfoodnetwork.org.au labels Aug 29, 2024
@wandji20
Copy link
Contributor Author

Hello @drummer83
I'm sorry for not getting back to you sooner, and
thanks for testing this one.

There is an additional popup from the browser now

It will seem we now block page exit if the order cycle form has unsaved changes.
However, this is new to me and I think some changes were made after I opened PR.
I will see if that's the case a find a suitable workaround

is it just due to the earlier conflict resolution?

Yes

@wandji20 wandji20 force-pushed the wb-OFN-12774 branch 3 times, most recently from 282c9e8 to 1a0ccbe Compare September 1, 2024 21:01
Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Cool, this could work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user facing changes Thes pull requests affect the user experience
Projects
Status: Test Ready 🧪
Development

Successfully merging this pull request may close these issues.

[Admin] [Order cycles] Cancel button of popup should redirect to OC list
6 participants