openfoodfoundation / wishlist

This repository welcomes ideas and suggestions to improve the OFN software.
3 stars 0 forks source link

[Admin] Don't jump to primary details after saving a change in Enterprise settings #316

Open audez opened 2 years ago

audez commented 2 years ago

Description

In Enterprise > Parameters, if you make some changes in any of the tab and click update, the page reloads, but brings you back on Primary Details.

Expected Behavior

You should remain on the same tab.

Actual Behaviour

When editing parameters on any tab (for ex Address - /edit#!#address_panel - or Social - edit#!#social_panel), and clicking the "Update" button, it brings you back on Primary details tab (/edit#!#primary_details_panel)

Steps to Reproduce

  1. Go to Enterprise > Settings
  2. Click on "Address tab", change something, click "Update"
  3. See that you're landing on the "Primary details" tab

Animated Gif/Screenshot

Oct-09-2022 10-50-04

Workaround

You have to go back to the tab you were in, it's annoying.

Severity

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

AthiraKadampatta commented 2 years ago

Hello @audez I would like to attempt this one. Could you please assign it to me?

drummer83 commented 2 years ago

Hi @AthiraKadampatta and welcome! I have assigned it to you! Let us know if you get stuck or if anything is unclear. Looking forward to your contribution! 🤗

RachL commented 2 years ago

@audez be cautious with the good first issue label, especially when it comes to Angular, it can be very tough. In doubt, ask a dev to add the label ;) Also I know this looks like a bug, but for all UX issue we should go through the wishlist process I think.

@AthiraKadampatta let us know how it goes. Note also that @binarygit is currently migrating all enterprise settings from angular to stimulus reflex. So I'm not sure it's worth spending time fixing this in Angular. @binarygit what do you think?

binarygit commented 2 years ago

I don't think it's a good idea to try and fix this in Angular :sweat_smile: . I think you'll need to look at the update method in app/controllers/enterprises_controller.rb because that's the action invoked after clicking update.

The view files for this screen are located in app/views/admin/enterprises, also app/views/admin/shared/_side_menu.html.haml renders the side menu and has the local anchor links such as #users or #primary_details.


There's some things I thought I'd share what I know about this here, maybe it'll help:

  1. The url (hash) is actually invalid: edit#!#primary_details_panel, there shouldn't be a #! in there but the pages render because the browser is forgiving.
  2. The #! probably comes from AngularJs but I could not track down which line of code causes this
  3. Valid url (hash) edit#primary_details_panel is automatically turned into edit#!#primary_details_panel by the browser

Also I have a rough idea of what might work:

  1. Get the url hash when user hits update
  2. The update action should use the url hash to render the correct panel with the correct side_menu selected. Consult this commit: https://github.com/openfoodfoundation/openfoodnetwork/pull/9510/commits/e18a242b903068121d8bf6fa2d93165ff112f3b2
RachL commented 2 years ago

Thanks for the feedback @binarygit ! Can this be already worked on right now with Stimulus Reflex or is it best to wait for your work to be completely done?

binarygit commented 2 years ago

oh yes. It can be worked on right now :smiley:

RachL commented 2 years ago

hello @AthiraKadampatta Just to be sure: are you working on this issue?

audez commented 2 years ago

I think it's the same problem but just to add this info: When you're in Enterprise settings and you reload the page, whatever tab you were in, it brings you back on "Primary details". And more boring: let's say you went in "Shipping methods" tab, you go in "Create a new shipping method", you click the previous arrow in your browser, and instead of being back to "Shipping methods" tab you land on "Primary details"

RachL commented 2 years ago

As no one is working on this I'm moving this to wishlist to review it during next papercut meeting.

AthiraKadampatta commented 2 years ago

Hi @RachL, Sorry I missed the above message. It took some time for me to understand Stimulus. I have tried to fix the issue in the draft PR - here. I have handled selection of the tab/panel based on the URL identifier.

I would like somebody to have a look at it before I finalize the PR. Thanks!