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

Replaced a lot of Magento logos with OpenMage ones #1599

Merged
merged 12 commits into from
May 18, 2022
Merged

Replaced a lot of Magento logos with OpenMage ones #1599

merged 12 commits into from
May 18, 2022

Conversation

fballiano
Copy link
Contributor

This repo has a lot of Magento logo images and some old OpenMage logos (different from the one on the new websites). This PR is trying to fix this situation as much as possibile:

  • replaced "/errors" pages logos with the new svg version
  • replaced "installation" logos with the new official version
  • replaced adminhtml themes logos with the new official version (sometimes with the svg, sometimes keeping the original formats for backward compatibility)
  • replaced all favicons using the new official one
  • replaced RWD theme logos with the new official version (keeping the original formats for backward compatibility)

I know it is a big PR but this project deserves to have a coherent image.

Also, because magento 1 didn't have any support for "2x" images (at least in logos) we should really migrate everything possible to SVG, but that would be for 20.x only, thus I left any unsave SVG conversion out of this PR, thus I think it's safe to merge it on the 1.9 branch.

Some screenshots:
Schermata 2021-05-05 alle 23 43 43
Schermata 2021-05-05 alle 23 37 24
Schermata 2021-05-05 alle 23 00 30
Schermata 2021-05-05 alle 23 00 28

Fixed Issues (if relevant)

  1. Fixes Magento logo in error display #1589

@github-actions github-actions bot added Component: Adminhtml Relates to Mage_Adminhtml Component: Page Relates to Mage_Page errors Relates to error pages Template : admin Relates to admin template Template : base Relates to base template Template : default Relates to base template Template : install Relates to install template Template : rwd Relates to rwd template labels May 5, 2021
Copy link
Contributor

@Flyingmana Flyingmana left a comment

Choose a reason for hiding this comment

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

While I may not be objective here, as I was introducing the logo you replace here, I do veto for a reason.
This is a big change in Branding. When I did introduce the Logo, I tried to get as many opinions and votes as possible. There were two alternatives back then offered. So it was a community decision back then, and if we do change the branding everywhere with the new logo, we definitely should make this a community decision, not a maintainer decision.

@fballiano
Copy link
Contributor Author

fballiano commented May 6, 2021 via email

@Flyingmana
Copy link
Contributor

@fballiano which of the possible answers would speak against making this a community decision?

@fballiano
Copy link
Contributor Author

it should be a community decision, I love community decisions.

but at this moment the non coherent branding is a bad thing for the project.

I also have to say that the new logo is already in the source code and the website, just not everywhere.

@addison74
Copy link
Contributor

addison74 commented May 6, 2021

My opinion:

  1. OpenMage needs a consistency of logos used in both Frontend and Backend. Also in default themes. All Magento logos need to be replaced. In Frontend I see it in error report pages, in Backend I see it when logging in and at the top left.

  2. The idea of using SVG is not bad at all. If I don't like the color, I can make an immediate change without taking the image to an editor and compressing it, which will definitely not looking good . The fact that the logo is SVG will make it easier to replace it in the future.

  3. If I could consider the first two mandatory, what the community can determine is whether OpenMage needs a new logo that conveys "something." The chosen color, the text, are vital. Colors like green or blue are preferred because they represent health and tranquility. It can be purple too. I do not agree using gradients.

@addison74
Copy link
Contributor

As you know there is a theme for Backend that is set by default. Whoever wants the Magento theme can set it in Settings. If a new color is set for the OpenMage logo then this color will be the main one. All the colors used in the theme must be analyzed with a tool like this one: https://color.adobe.com/create/color-wheel. Thus we create a beneficial harmony for our eyes, being a pleasure to stay in the interface sometimes for a longer time.

@colinmollenhour
Copy link
Member

Thanks for doing all of the lifting on this @fballiano!

I think everyone agrees on replacing the Magento logo, so that's good. 👍

I suppose when we redesigned the public site and introduced the new logo that caused this confusion so I apologize for that. The intent was to get a professional designer to give the public site a face-lift keeping the spirit of the original intact to whatever degree was appropriate. I/we gave the designer a lot of freedom in this and I think the result was fantastic. The community also only had great things to say about the new design when it was previewed before it was merged so I thought there was already more or less a consensus on the "new" logo more or less replacing the "old" logo? I never set out to "change" the logo but effectively I suppose we did and although it is different it is also quite similar, with dropping the gradient for a solid color being the only major difference (to my eyes). Kudos to @Flyingmana for designing a logo that was able to survive a complete site redesign with minimal changes and still look great!

I personally am not opposed to keeping the old logo in the original admin theme and having the new one in the new theme, it's not too uncommon for old designs to linger in some places. Neither logo looks good with the old Magento admin theme's pale green and orange tones.. 😄

But I can certainly see the argument for having one consistent brand to present everywhere and so would agree with going ahead and replacing it with the new one as Fabrizio has done with this pull request. So +1 for merging from me.

@Flyingmana
Copy link
Contributor

One correction, I did not design the logo, I did had a Designer friend commissioned for one.
So the Kudos belong to Nirit Nagar https://www.xing.com/profile/Nirit_Nagar

@fballiano
Copy link
Contributor Author

just a couple of notes:

  • the new logo is awesome
  • although it seems weird to see in on the legacy adminhtml theme, it's more readable/visible than the old but most of all I'm not comfortable enabling the new default adminhtml for my customers, it's a nice theme but the text/background contrast and the color scheme makes it not easy to read while the legacy theme (although not really modern looking) doesn't have this problem (the backend IMHO has to be really usable, form over design for sure). sadly I've no competence to improve the graphic design of the new adminhtml theme.

@addison74
Copy link
Contributor

If it were up to me I would make the theme for Backend a separate project. Thus, people will be found to contribute ideas and improve it, including a specially chosen color palette, fonts, etc. OM will be left with code changes so that those who want to install the theme to use it instantly. I appreciate the effort for this theme but all the users in the installations I manage have asked me for the original Magento version.

errors/default/page.phtml Outdated Show resolved Hide resolved
skin/adminhtml/default/default/boxes.css Show resolved Hide resolved
@luigifab luigifab mentioned this pull request May 16, 2021
3 tasks
@fballiano
Copy link
Contributor Author

Admins do we have to do to move this forward?

@fballiano
Copy link
Contributor Author

is the actual one:
Schermata 2021-07-04 alle 14 47 16

really considered better than the SVG one? The OM symbol it's ok but the "Admin Panel" font looks awful, please let's get rid of it!

@fballiano fballiano requested a review from Flyingmana July 4, 2021 12:49
@addison74
Copy link
Contributor

@fballiano - I totally agree. The text doesn't look good at all. It seems the letters spacing is different and also the font used. I am wondering if "Admin Panel" is mandatory as we all know this is OM Backend. Log + OpenMage should enough.

@fballiano
Copy link
Contributor Author

Hi @Flyingmana, how could we move this forward?
We've 7 positive reviews on this PR (the most reviewed open PR at the moment) but you were talking about community vote. Although I think the new logo is just a better version of the old one I see your point, but how can we do that? Also, how could we measure if a "vote" is actually successful?

@fballiano
Copy link
Contributor Author

Sorry to bother but we've 6 positive reviews on this PR and it's vetoed since months, what are we doing on this?

@colinmollenhour
Copy link
Member

I think @Flyingmana's change request was the only hold-up but I'm not sure if he is still objecting or just didn't dismiss his change request..

we definitely should make this a community decision, not a maintainer decision

It appears to me the community has spoken as there are no arguments made against the change (Daniel's objection seems to be more a procedural one and not a substantive one) and many approvals.

@colinmollenhour colinmollenhour merged commit 3ba2ead into OpenMage:1.9.4.x May 18, 2022
@github-actions
Copy link
Contributor

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
7 runs  +1  5 ✔️ +1  2 💤 ±0  0 ❌ ±0 

Results for commit 3ba2ead. ± Comparison against base commit 8b0c0b9.

@luigifab
Copy link
Contributor

luigifab commented Jun 6, 2022

Not sure, but it seems that old emails sent appear with a bad logo:

image
image

This is only my history?

@fballiano fballiano deleted the issue_1589 branch June 6, 2022 20:21
@addison74
Copy link
Contributor

@luigifab - Most likely the template used displays the current image on the server not the old one.

Copy link
Contributor

@S0FTWEX S0FTWEX left a comment

Choose a reason for hiding this comment

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

Changes in file skin/adminhtml/default/default/boxes.css (#1599) was not merged into 20.0.15

@fballiano
Copy link
Contributor Author

@Flyingmana it really seems that @S0FTWEX is right, on branch 20.0 skin/adminhtml/default/default/boxes.css doesn't reflect the changes from this PR. how could we solve this?

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: Page Relates to Mage_Page errors Relates to error pages Template : admin Relates to admin template Template : base Relates to base template Template : default Relates to base template Template : install Relates to install template Template : rwd Relates to rwd template
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Magento logo in error display