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

[Spree Upgrade] Fix "back to list" button overlap and remove configurations menu from payments methods list #3648

Merged
merged 2 commits into from
Apr 10, 2019

Conversation

luisramos0
Copy link
Contributor

What? Why?

Closes #3543

In this PR we fix the button layout issue 3543 and we remove the configurations menu from the payments methods list.
@RachL I think you said it was good to have the configurations menu back but we can't because this page is also seen by non-admins coming from the enterprise management page (they would see the menu and all the links would take the user to the "no authorization" page). You can create an issue to detect if user is admin or not and show the menu if user is admin, OR show the menu if user is coming from the configurations and not from enterprise management.

What should we test?

  • "back to list" buttons do not overlap.
  • payment methods list doesnt show the configurations list.

@luisramos0 luisramos0 changed the title 2 0 fix ui issue [Spree Upgrade] Fix "back to list" button overlap and remove configurations menu from payments methods list Mar 24, 2019
@luisramos0 luisramos0 self-assigned this Mar 24, 2019
…s list, this is necessary because non-admin users do get to this page through enterprises management"
Copy link
Member

@kristinalim kristinalim left a comment

Choose a reason for hiding this comment

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

Nice, @luisramos0! 🙂

@RachL RachL self-assigned this Apr 10, 2019
@RachL RachL added the pr-staged-fr staging.coopcircuits.fr label Apr 10, 2019
@RachL
Copy link
Contributor

RachL commented Apr 10, 2019

I think you said it was good to have the configurations menu back but we can't because this page is also seen by non-admins coming from the enterprise management page (they would see the menu and all the links would take the user to the "no authorization" page).

Ok this makes sense. To be honest I would prefer us to work on the steps sometimes : to me it is super weird that the enterprise manager ends up on the global payment method page. When the enterprise users clicks on the go back button he should be redirected to the admin/enterprises/enterprisename/edit#/payment_methods page. Thus super admin and admin have two different pages, and we reduce the number of clicks for the manager. And no need to hide the left menu for the super admin. So this is something I have to put on discourse I guess.

Buttons are good:

image.png

And no left menu (both super admin and admin) :

image.png

Ready to go!

@RachL RachL removed the pr-staged-fr staging.coopcircuits.fr label Apr 10, 2019
@luisramos0
Copy link
Contributor Author

nice @RachL makes sense! discourse or gh... it's an improvement.

@luisramos0 luisramos0 merged commit 9841550 into openfoodfoundation:2-0-stable Apr 10, 2019
@luisramos0 luisramos0 deleted the 2-0-fix-ui-issue branch April 10, 2019 20:46
@RachL
Copy link
Contributor

RachL commented Apr 11, 2019

@luisramos0 alright! Will open it this way

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

Successfully merging this pull request may close these issues.

4 participants