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

add OpenMage admin theme and theme switcher #1008

Merged
merged 22 commits into from
Aug 20, 2020
Merged

add OpenMage admin theme and theme switcher #1008

merged 22 commits into from
Aug 20, 2020

Conversation

aterjung
Copy link
Contributor

in addition to logo integration build a draft version of a new styled backend-theme for OpenMage. I think it is more and more important to show the ongoing development to the users. I would like this to be seen as a first step - a new Backend-Style mainly build by css changes on top of the old default/default admin theme to be as compatible as possible. I my mind, there should follow a second step, with a more up-to-date approach.

This PR consists of to parts, the theme switcher to switch from OpenMage Theme to the legacy Magento M1 theme, and the theme itself.

  • For the theme switcher, is accepted to build it into the core, or should i build it as a new module?

The theme itself supply only one phtml file to change the login, because that can not be done via layout files. For the themes CSS i used some parts from https://github.com/jreinke/magento-admin-theme and converted them to SCSS. I have replaces as many icons as possible to inline-SVGs.

Help with the styling would be very welcome!

@Schrank
Copy link
Contributor

Schrank commented May 27, 2020

Would you mind to add a screenshot? I would love to get an impression without installing it 😅

@aterjung
Copy link
Contributor Author

no problem, here are some screens!
Bildschirmfoto 2020-05-27 um 21 10 42
Bildschirmfoto 2020-05-27 um 21 11 43
Bildschirmfoto 2020-05-27 um 21 12 22
Bildschirmfoto 2020-05-27 um 21 12 56
Bildschirmfoto 2020-05-27 um 21 13 09

@aterjung
Copy link
Contributor Author

what i did not ask in my comment, is this a change for the 19.4 branch, because it is a non breaking change, or should it go to the 20.4 branch because is something new? At the 20.4 branch, i don't see progress at the moment?

@colinmollenhour
Copy link
Member

Looking great!

Please make sure you include somewhere instructions on generating the CSS files from the SCSS files so that anyone may easily add updates and not have to figure out the tooling from scratch. A comment in the generated CSS file to prevent users from directly editing them would be good to avoid confusion.

@colinmollenhour
Copy link
Member

Forgot to address the branch question.. You have it on the right branch. How the version is tagged after this is merged I think I would defer to @Flyingmana.

Upon upgrade it would be nice if this is the default skin as long as users can go back to the old one in case it breaks their extensions somehow.

@Flyingmana
Copy link
Contributor

if there is no BC break, adding it to 1.9.4.x sounds good enough.

Regarding 20.x , I need to add a note to merge the changes upstream and also do a release there <.<

@aterjung
Copy link
Contributor Author

i have added a comment to prevent editing the css files. Also i have replaces a lot of old looking icons with svg versions - but still everything via css. From my point of view, that should be BC, but it has to be tested with the usual extensions.

@colinmollenhour its allready build that way, the OpenMage theme is as standard theme, but the old legacy theme can be selected from backend.

@sreichel sreichel added new feature rebranding Change Magento to OpenMage LTS labels Jun 1, 2020
@colinmollenhour
Copy link
Member

Any reasons not to go ahead and merge this? I loaded it up and tested it out and everything works well.

@luigifab
Copy link
Contributor

luigifab commented Jun 7, 2020

Is the theme can be set per admin user? (in System / My account).

@aterjung
Copy link
Contributor Author

aterjung commented Jun 7, 2020

i tested it myself in some customer installations without problems. From my side, it would be ready to merge. I will change the state!

@aterjung aterjung marked this pull request as ready for review June 7, 2020 13:25
@colinmollenhour
Copy link
Member

Is the theme can be set per admin user? (in System / My account).

No, that is not a feature currently.

@aterjung
Copy link
Contributor Author

Is the theme can be set per admin user? (in System / My account).

no, its selected for all users, from my point of view, it should not be selectable by individual user. Its not the intension to open a complete new playground for X Admin themes with Y incompatibilites. Its meant to give OpenMage its own look and identity. But you could switch back if there are Problems with extensions.

@aterjung
Copy link
Contributor Author

I have updated the color theme a bit to reflect the upcoming new website. I hope you like it!
Only Color-Values changed, def. no BC!
Bildschirmfoto 2020-06-11 um 00 24 53

@sreichel
Copy link
Contributor

I'll test on saturday!

There is no default config value for Mage::getStoreConfigFlag('admin/design/theme') .

    <default>
        <admin>
            <design>
                <theme>0</theme>

.... and maybe rename it to use_legacy_theme to make it more clear what the option is for.

sreichel
sreichel previously approved these changes Aug 5, 2020
Copy link
Contributor

@sreichel sreichel left a comment

Choose a reason for hiding this comment

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

Dont want to wait for last minor fixes.

@aterjung aterjung dismissed stale reviews from sreichel and colinmollenhour via c38cec0 August 5, 2020 10:05
@aterjung
Copy link
Contributor Author

aterjung commented Aug 5, 2020

as far as i can see, every mentioned issue is solved now? I will add optimisations in the future, but i think it should not block merging now.

Copy link

@Adel-Magebinary Adel-Magebinary left a comment

Choose a reason for hiding this comment

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

Looks okay to me.

Copy link
Contributor

@sreichel sreichel left a comment

Choose a reason for hiding this comment

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

Nice work 👍

Not blocking, but ...

  • i dont like the default yellow admin notifications :P
  • "Global Record Search" results are displayed on full width (IMHO default width was bit to small, but full width ins't better)

@sreichel sreichel removed this from the Release 20.0.1 / 19.4.5 milestone Aug 7, 2020
@aterjung
Copy link
Contributor Author

anything i should change? I intended to stop any further development until this state is merged?

@colinmollenhour colinmollenhour merged commit 69fa3aa into OpenMage:1.9.4.x Aug 20, 2020
@colinmollenhour
Copy link
Member

Great work everyone! Please feel free to submit separate PRs for improvements.

@ioweb-gr
Copy link
Contributor

Could this be tagged as a minor release? I'd love to try it

@colinmollenhour
Copy link
Member

Oops, I didn't realize there was a 20.0 branch.. I just merged it into the 20.0 branch. @Flyingmana do you want me to revert it from 1.9.4.x? I'll leave the release strategy up to you.

@sreichel
Copy link
Contributor

@colinmollenhour IMHO 1.9.4.x is correct ... (or is it clear what the 20.x branch is for?)

@sreichel
Copy link
Contributor

anything i should change?

After it is merged ... see my comment above ;)

Thanks for your work 💯

@sreichel sreichel added this to the Release 19.4.7 / 20.0.3 milestone Aug 20, 2020
@Flyingmana
Copy link
Contributor

@colinmollenhour
If it has a potential to cause issues, then 20.0, else its fine as it is.

@addison74
Copy link
Contributor

After 12 years of using M1 it is difficult to ask people to adapt to other backend theme. We should be patience with everyone. This new theme should be a proposal not an imposition. Personally I will advice anyone to test it before using it. It could be a package installed by those who want it.

Related to the style of the theme I agree we need a SCSS file to change easy the values. Many programmers are not UX designers. I suggest a limitation in using colors and please use a Color Wheel to choose correctly the color pallet. You may use Adobe Color Wheel from here: https://color.adobe.com/create/color-wheel. Or ColorImpact from here: https://www.tigercolor.com/. Probably the colors look OK for the eye but the brain could have some issues with the chosen ones. I would like to see a different logo for OpenMage to express an idea not an m with a text.

This is a personal opinion and should be taken as such ...

@LuizSantos22
Copy link

I would give as a inspiring idea for new admin theme release, the extension created by CMSmart Admin Theme for Magento 1
I used to use it when I was still using "pure" magento. But after upgrading to OpenMage, it has stopped working
and I did not bother trying ways to fix it.
This is their link: http://preview.cmsmart.net/demo/magento-responsive-admin-template (Magento 1 is on the left)

It has icons on the menu names and it can be set to be horizontal (like normal magento) and vertical (like magento 2).
In my opinion, vertical menu is more elegant than horizonal one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: Core Relates to Mage_Core Component: Page Relates to Mage_Page new feature rebranding Change Magento to OpenMage LTS Template : admin Relates to admin template
Projects
None yet
Development

Successfully merging this pull request may close these issues.