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

Bring basic CSS rules from spree backend #4620

Merged
merged 6 commits into from
Feb 4, 2020

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented Dec 20, 2019

What? Why?

Continues #4595

Bring basic css from spree_backend.

What should we test?

I think we should compare visually some of the main pages of the backoffice to an instance without this PR and try to verify that the pages look the same in terms of style: size of components, position of components and color of components.

Release notes

Changelog Category: Changed
Brought all the basic css rules from spree_backend so that we can make ofn independent of Spree.

@luisramos0 luisramos0 changed the title Spree backend css Bring basic CSS rules from spree backend Dec 20, 2019
@luisramos0 luisramos0 self-assigned this Dec 20, 2019
Copy link
Contributor

@Matt-Yorkley Matt-Yorkley left a comment

Choose a reason for hiding this comment

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

If we want to update the admin CSS at some point, it might be helpful to separate/namespace the imported spree CSS from the customisations we've added on top of it...? Just a thought.

@luisramos0
Copy link
Contributor Author

yeah, I considered that option. I kept the spree JS code in a separate folder under admin/spree because it's not angularized, very different from ours. But in CSS from now on I thought we should probably just mix everything and start changing the rules in these files if needed and merge these files with our files if we have the opportunity. Like merging admin/variables and new admin/globals/variables.

@luisramos0 luisramos0 force-pushed the spree_backend_css branch 2 times, most recently from 5edb786 to ddbd9fd Compare January 24, 2020 17:50
@luisramos0
Copy link
Contributor Author

I just rebased and tested the deployment process (I had to add some imports in the css files to make the deployment work. All good now.
Ready for testing.

@lin-d-hop lin-d-hop added the pr-staged-fr staging.coopcircuits.fr label Jan 30, 2020
@lin-d-hop
Copy link
Contributor

Compared FR staging to UK production
Checked top and second level menu pages.
Checked all Edit sections (order, product)
Checked Enterprise settings
Everything looks as pretty as ever.
Good to go!

@lin-d-hop
Copy link
Contributor

@luisramos0 conflicts

@lin-d-hop lin-d-hop added feedback-needed and removed pr-staged-fr staging.coopcircuits.fr labels Jan 30, 2020
@luisramos0 luisramos0 force-pushed the spree_backend_css branch 2 times, most recently from 454d1ae to 7df8a5e Compare January 30, 2020 11:45
@luisramos0
Copy link
Contributor Author

Some conflicts related to the order the css files are imported.
I think we should have a quick look at this one again.
Just making sure no major problems appear, for example, in the list of orders in admin/orders.

@luisramos0
Copy link
Contributor Author

I rebased again. I think we can just deploy and verify that are no display errors in the orders list page.

@filipefurtad0 filipefurtad0 self-assigned this Feb 4, 2020
@filipefurtad0 filipefurtad0 added the pr-staged-fr staging.coopcircuits.fr label Feb 4, 2020
@filipefurtad0
Copy link
Contributor

Hey @luisramos0,

Staged this PR in FR and compared it with AU in production.
All seems the same, so moving to ready to go!
Thanks!

@filipefurtad0 filipefurtad0 removed the pr-staged-fr staging.coopcircuits.fr label Feb 4, 2020
@luisramos0 luisramos0 merged commit c7038f6 into openfoodfoundation:master Feb 4, 2020
@luisramos0 luisramos0 deleted the spree_backend_css branch February 4, 2020 18:06
@luisramos0 luisramos0 mentioned this pull request Feb 6, 2020
18 tasks
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.

None yet

5 participants