Page MenuHomePhabricator

Build new logo for Desktop Improvements Header
Closed, ResolvedPublic5 Estimated Story Points

Description

Background

This task will cover the creation of the new logo for the header, following the guidelines created in T245190: Implement guidelines around logos creation. This will allow us to update the logo and continue with the remainder of the planned features, such as the collapsible sidebar.

After determining how to structure the new logo file itself T244486, we have to place the logo into the header for the new desktop improvements experience.

Prototype

DEMO (patch 585358)

Design
legacy VectorVector Neue

A few design details (source): The logo should be left-aligned with the text in the sidebar. The content can be pushed down slightly to give the horizontal logo more breathing room.

  • header height: fixed at 54px
  • header padding-top: 10px
  • header padding-bottom: 5px
  • for now we can leave the alignment of the user-links as-is, which should result in them being top-aligned with the logo element
Technical notes:
  • The current Vector layout places the logo inside the sidebar (.mw-panel) instead of in the header. For the new experience, this changes means removing the old logo from the sidebar and adding the new logo into the header.
  • The header placement is technically below the content in Vector, but restructuring the header is not required for this task.
Acceptance criteria
  • the alt text of the wordmark is the project name
  • the alt text of a the icon is the project name with icon in brackets and subject to i18n
  • the tagline has alt text that is configurable per project
  • the site
  • if no wordmark is defined the site title will be used in its place
  • wordmark, icon and tagline are all optional. The absence of all three will show an h1 with the site title
  • patch has been signed off by performance team
  • old vector continues to show the old logo
  • a new ResourceLoader module will be needed to ship the css for old Vector. It will eventually not be loaded in new Vector
  • the header height has been increased in new Vector
  • the header height remains unchanged in legacy Vector
QA steps
  • the header height has been increased in new Vector
  • the header height remains unchanged in legacy Vector
  • there is no increase in CSS bytes for legacy Vector
  • the legacy Vector logo remains unaffected
  • the new Vector logo displays when I am using the new Vector version
  • There is a storybook entry that allows us to test different logo combinations
Sign off steps
  • Logo will be an image tag meaning changes to the logo will need to take into account caching and suboptimal CSS hacks such as hiding img tags with display none. Product has documented this.
  • Serve SVGs only with no PNG fallback. For grade C browsers we will display text.
  • Product has documented this pointing to the percentage of our users that have SVG support.
Developer notes

Some sample logos can be found here: T245190#5939228
Use the following config when testing:

$wgLogos = [
    'icon' => 'https://di-logo-sandbox.firebaseapp.com/img/globe.png',
    'tagline' => [
        'alt' => 'the free encyclopedia',
        'src' => 'https://di-logo-sandbox.firebaseapp.com/img/tagline/en-tagline-117-13.svg',
        'width' => 117,
        'height' => 13,
    ],
    '1x' => 'https://en.wikipedia.org/static/images/project-logos/enwiki.png',
    'wordmark' => [
        'src' => 'https://en.wikipedia.org/static/images/mobile/copyright/wikipedia-wordmark-en.svg',
        'width' => 116,
        'height' => 18,
    ],
];
Areas of risk
  1. We will need to load an additional stylesheet on the new Vector while not loading it on old Vector
  1. We will need to increase the header height in Vector. This will require lots of careful changes across multiple elements. The fact that this height depends on the logos also adds a lot of complexity given the absolute positioning of Vector. We will likely have to use flex box to get the vertical alignment and revisit the absolute positioning possibly relying on some sort of ResourceLoader LESS variable here. HERE BE DRAGONS
  1. We need to make a decision about how to separate old Vector and new Vector in templates. Personally I would suggest two master templates and a single config flag which chooses between them. The template data would be shared by the two templates to encourage us to generalise that code.
  1. Per @Krinkle use of flex box will mean we need to consider the grade C experience and how broken the experience looks. We may want to punt this to a separate task prior to releasing.

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

https://gerrit.wikimedia.org/r/c/585629 already implemented pixel-perfect vertical centering of PersonalMenu in the Header.

Patch https://gerrit.wikimedia.org/r/586451 fails to do vertical centering. Absolute positioning with arbitrary browser font-size is unable to do that.
Unfortunately, the patch causes significant merge conflicts in 4 patches. @Jdlrobson, @Niedzielski and me have been working together on those patches for days. These merge conflicts waste our time for no benefit. Please revert the patch to avoid the damage.

This patch was merged... 1 and half hours after being submitted. The only review was by a developer, who was uninvolved with development and reviews in the DI project in the last weeks.
The review process is strict and involves multiple developers to avoid these mistakes.

@Jdlrobson one day ago you have written we aren't touching the PersonalMenu at this stage...
What was the thinking here to contradict your own statement and do this?

To make progress, we shall finish the outstanding patches that @Niedzielski, @Jdlrobson and me have been working on for days. Multiple patches were ready for review for days, waiting for your input and follow-up. Could we focus on that?

Patch https://gerrit.wikimedia.org/r/586451 fails to do vertical centering. Absolute positioning with arbitrary browser font-size is unable to do that.
Please revert the patch to avoid the damage.

This patch moves a value into a variable. It doesn't change the status quo. Additionally it lets us make the changes to the header without touching any code that runs in the legacy version. It's not going to be reverted.

This patch was merged... 1 and half hours after being submitted.

Yes as the result of a pair programming session between two Wikimedia developers who have over four years experience with this codebase.

@Jdlrobson one day ago you have written we aren't touching the PersonalMenu at this stage...
What was the thinking here to contradict your own statement and do this?

We're still not touching personal menu. The variable has been hoisted out of the file so that I don't have to touch it any further. The latest version of https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/577682 has now been updated so it doesn't have to touch the legacy version (aside from a strategic rename to "freeze" the template which is still open for discussion. I'll continue on that patchset tomorrow.

Demian added a comment.EditedApr 7 2020, 4:38 AM

This will be long. I'll try to explain everything that's wrong with this patch in enough detail, so it can be understood.

Additionally it lets us make the changes to the header without touching any code that runs in the legacy version.

As I have demonstrated in the fully working DEMO (patch) 2 days ago, there is no need to do this to "make changes to the header without touching any code that runs in the legacy version".

It's not going to be reverted.

This patch conflicts with and breaks a chain of 6 patches and @Niedzielski's patches. The merges will take at least half an hour and then the CI tests another half hour, then testing both legacy and new on each patch. 1-2 hours wasted for one LESS variable that will be removed in a few patches.

If you stick to this commit, then please delay it until @Niedzielski's and my patches are reviewed and merged. That way our time wouldn't be wasted and you could continue with it, when the time comes. As you have said: "PersonalMenu comes later".

All this for a patch that does not contribute to vertical centering.

This patch was merged... 1 and half hours after being submitted.

Yes as the result of a pair programming session between two Wikimedia developers who have over four years experience with this codebase.

I didn't say lack of experience is the problem. The problem is ignoring and wasting others work. I'm stunned that you would ignore others efforts and investment and rapidly merge in a breaking patch behind our backs, without consensus. 4 patches - waiting Your review - were listed as conflicting and that did not matter? This is not what you promised, not what we agreed upon. You haven't even informed or asked us, so we could find the least painful solution?
At least when you are asked to take into consideration other's effort and work... could you please listen to the people who work with you?

I've been very patient to how 80% of my work was wasted in the last months, but this is completely unreasonable, so now I ask you to mitigate the damage.

@Jdlrobson one day ago you have written we aren't touching the PersonalMenu at this stage...
What was the thinking here to contradict your own statement and do this?

We're still not touching personal menu.

I have similarly "not touched" the PersonalMenu and you've rejected the patch without even reading it: https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/585367#message-b2f16e82c2ab66639e6e4c30632a10b7baa46a55
A little consistency would make communication more efficient and our work more fruitful.

The variable has been hoisted out of the file so that I don't have to touch it any further.

I see: you want to use the same PersonalMenu.less file in both legacy and new. Once PersonalMenu will be the task to do, 50% of the PersonalMenu.less file will be rewritten and hoisting the top-* variable will become unnecessary. I've made that patch already and I've show a working demo. You could have just merged it. Hoisting this variable gives no lasting benefit, but it causes a lot of disruption, that could be easily avoided.

In fact, the 2nd patch I've made did all this in one step, but you rejected it, writing: "PersonalMenu is coming much later. Please revert back to the previous approach...". I reckon the patch was not trivial to understand, but I would have explained it line-by-line if you ask.

The latest version of https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/577682 has now been updated

That patch is still full of lint errors, follows an old version of the logo spec with incorrect sizes, the CSS can be simplified significantly and it gives a subpar result on IE11 because of the @supports solution I've introduced in an early patch, that I don't use anymore.

To fix all these issues, I've rebased, fixed and cleaned up your patch. You only had to download it and write your Change-Id into the commit message:
https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/585358/12

This was the case for a few days by now, but the only thing that has happened is this unnecessary patch which breaks all that we've been working on.

@Demian: Please read and follow the Etiquette, especially "Criticize ideas, not people", if you would like to be active here. Thank you.

Demian added a comment.EditedApr 7 2020, 5:07 PM

@Demian: "Criticize ideas, not people"

@Aklapper I've extensively criticized the idea to merge an ephemeral one-liner patch that breaks 7+ significant patches we've been working on, all this without discussion and consensus. The problem is not specific, however, to one person, but generally, the reviewing practices that wasted most of my volunteer efforts in 2 months' free time, somewhere around 100+ manhours. Please clarify what you find wrong with raising a complaint about this and I'll adjust the comment. Feel free to answer off-ticket.

all this without discussion and consensus

Whether to merge a patch is at the discretion of the maintainers of that repository. That's the only consensus that matters and Volker and I are two of the maintainers in this repository.

Please reflect on what your asking here for a moment. If project maintainers who are working full time on a project looked for consensus from volunteers on every single trivial patch nothing would get done. The RFC process was designed as a vehicle to get feedback where it's deemed useful and this is not one of those cases.

My work here is blocked on T249073 and T249372 - there are lots of small conversations going on in those two task which are making it hard to make this change with a level of risk I feel comfortable with.

@Jdlrobson I had to spend more than 2 hours to do all the merges that I wanted to avoid with the above complaint. The results:

  1. As you've requested, I've reduced the Header component patch to the bare skeleton: 585367 (DEMO).
  2. I've rebased your Logo patch on it: 587613.
  3. And updated my patch to demonstrate all the suggestions: the fixes, the CSS simplified, IE11 supported: 585358 (DEMO).

We'll see if it was worth to do it, or this also goes to the long list of ignored patches.

Change 587613 had a related patch set uploaded (by Aron Manning; owner: Jdlrobson):
[mediawiki/skins/Vector@master] A new version of Vector with a new logo

https://gerrit.wikimedia.org/r/587613

My work here is blocked on T249073 and ...

Thank you for your conclusions there, we are getting close to a consensus. I've updated the Header element and styles patch (585367) according to the most likely outcome (it's no longer a template / component) and rebased your Logo patch on it (587613).

Change 588461 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] Mark layout code as legacy

https://gerrit.wikimedia.org/r/588461

https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/588461 is ready for review. This will allow us to make isolated changes to the header for new vector.

Demian updated the task description. (Show Details)Apr 15 2020, 3:09 AM

Change 588461 merged by jenkins-bot:
[mediawiki/skins/Vector@master] [legacy] Split sidebar code and mark layout as legacy in preparation for new layout

https://gerrit.wikimedia.org/r/588461

Next step would be to 1-on-1 with Alex to talk through some things.

@Demian as the main lead on this ticket I respectfully ask that you please don't associate new patches with this task. It's creating a lot of confusion to my reviewers and really slowing me and progress in this task and project as a whole. Ideas are appreciated, but I will ask for them as and when needed and right now. I am the assignee field in Phabricator, please respect that. When you don't respect that it makes collaboration near impossible.

Unfortunately Phabricator's Gerrit integration doesn't update once patches are removed so now we have many patches listed at the top of this ticket which is making it confusing for people to know what to review. Let me clarify now.

The only patches that should be associated with this task now are A new version of Vector with a new logo and Aron's patch Add Header element and styles which is a subset of the former. Aron, please focus on helping me get the latter patch into a good place if you want to help me.

@alexhollender and I will meet to talk about the task on Monday. Reviews on A new version of Vector with a new logo are welcome.

Change 578674 restored by Jdlrobson:
Add Header template skeleton

Reason:
Restoring to remove bug number to clear up phabricator confusion.

https://gerrit.wikimedia.org/r/578674

Demian added a comment.EditedApr 17 2020, 5:29 PM

@Demian please don't associate new patches with this task.

Sure. You could have just asked...

It's creating a lot of confusion to my reviewers and really slowing me and progress in this task and project as a whole.

The last one was a week ago: T246170#6045330. I don't understand your wording, how does it confuse after a week?

If anything, you could save weeks by considering at all to merge the patches I make weeks in advance. Please refrain from blaming me for the inefficiencies. As Aklapper pointed out, Phabricator is no place for criticism of people, especially not falsely. If you wish, I'm happy to discuss this on-wiki.

I am the assignee field in Phabricator, please respect that. When you don't respect that it makes collaboration near impossible.

You can just simply say that these patches should be added to another ticket. You've never said that before. It's a form of respect to not assume bad faith.

The difficulty of collaboration comes from a lack of appreciation and general ignorance towards volunteer efforts. I don't complain either about that and the regular disrespect I receive.
I respectfully ask you to not point fingers at me. Thank you.

Unfortunately Phabricator's Gerrit integration doesn't update once patches are removed so now we have many patches listed at the top of this ticket which is making it confusing for people to know what to review. Let me clarify now.

In 5 minutes the cache will update...

The only patches that should be associated with this task now are A new version of Vector with a new logo and Aron's patch Add Header element and styles which is a subset of the former.

That's fine.

Aron, please focus on helping me get the latter patch into a good place if you want to help me.

It's worth noting that I've spent dozens of manhours focused on that patch in recent weeks. A new PS is soon ready. Maybe you can say thank you one day instead of this...

@Jdlrobson Do you think my patches would be better suited to the parent ticket?
T240856: [EPIC] New header for desktop improvements project
I wasn't sure about that when I uploaded these.

As Aklapper pointed out, Phabricator is no place for criticism

[OT] @Demian: Please do not put words into my mouth. Nobody ever said that "Phabricator is no place for criticism". If you opened and read the page that you linked, then you see that it says "Criticize ideas, not people". Furthermore there is a difference between requests (such as "Please respect that") and accusations (such as "This is not what you promised" or "You haven't even informed or asked us"). Thank you.

Jdlrobson added a comment.EditedApr 17 2020, 6:20 PM

The difficulty of collaboration comes from a lack of appreciation and general ignorance towards volunteer efforts. I don't complain either about that and the regular disrespect I receive.

It works both ways. I appreciate volunteer efforts and my Gerrit record will show that over the past 8 years on this project I am very generous with my time on volunteer patches.

The fact many of us are "inefficient" as you put it, is likely due to the fact that there is a global pandemic and many peoplehave different ways of dealing with that. I am not criticizing you here nor blaming you but respectfully attempting to feedback to you in good faith that your actions are causing some challenges.

It's worth noting that I've spent dozens of manhours focused on that patch in recent weeks.

I have also spent many hours writing patches and trying to reach consensus in conversations and replying to yourself in various places. Consensus doesn't come cheap. If you don't like the speed at which we're working than this is probably not a code-base you are going to enjoy working in. Repeatedly complaining about the time you have wasted to the people that maintain code does not help progress the conversation.

The challenge you are facing is one of process. Usually the process for contributions is as follows: 1) person comments that they would like to work on a ticket 2) maintainer quickly responds with their thoughts and whether they have bandwidth to review 3) person assigns themselves to a Phabricator ticket setting out an intention prior to writing patches. 4) patchset is posted 5) the two collaborate until there is a shared understanding of the problem and the patch is merged. Steps 1 and 2 can be skipped for any task which has good first task or patch-welcome on it. I see you are active on various other tasks which are being worked on by others and this is likely why you are spending time on things that are not seeing fruition. Find a bug that nobody's working on and ask if you can work on it. I guarantee your experience of code review will be much smoother.

@Jdlrobson Do you think my patches would be better suited to the parent ticket?
T240856: [EPIC] New header for desktop improvements project
I wasn't sure about that when I uploaded these.

A patchset doesn't always need to reference a phabricator ticket in other ways so that it doesn't link to the patch e.g. "This patch is an example for how T00000 could be done." in the commit message or just a comment on the phab ticket/gerrit.

Change 585358 had a related patch set uploaded (by Aron Manning; owner: Jdlrobson):
[mediawiki/skins/Vector@master] [DNM-SAMPLE] [modern] Add Logo component

https://gerrit.wikimedia.org/r/585358

Change 585629 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] [modern] Align vertically the PersonalMenu with the new Logo

https://gerrit.wikimedia.org/r/585629

Sorry for the noise.

[OT] @Demian: Please do not put words into my mouth. Nobody ever said that "Phabricator is no place for criticism".

@Demian: Please read and follow the Etiquette, especially "Criticize ideas, not people", if you would like to be active here. Thank you.

I'm sorry @Aklapper for not being clear on that: as I was being criticized, I obviously meant "criticism of people". I've updated my comment to clarify this. Thank you for the feedback, although I do not understand the intensity of your reaction.

[OT] As you've requested this from me previously, I wonder if you will ask JdlRobson to follow the Etiquette. I assume not, although this is not the first occasion. Not that I want that, just pointing out the apparent bias. Respectfully, please understand that I find these comments shifting blame to the volunteer. This is not a healthy pattern of collaboration and I'm making much more effort to contribute positively than how you depict. The rest of your comment I've refuted previously on your talk page. Please continue this discussion there. Thank you.

A patchset doesn't always need to reference a phabricator ticket in other ways so that it doesn't link to the patch e.g. "This patch is an example for how T00000 could be done." in the commit message or just a comment on the phab ticket/gerrit.

I don't understand this sentence, could you clarify?

The Header element patch is up.

Demian updated the task description. (Show Details)Apr 19 2020, 4:33 AM
Jdlrobson updated the task description. (Show Details)Apr 20 2020, 7:19 PM

I chatted with @alexhollender today about this task here are some notes from our chat and agreements:

  • The header container should be 54px amd the icon in the logo should be 50px tall. Will fix this up later today.
  • We agreed to remove the gradient from new Vector and make the background of new Vector white (drop the f6 color). I will need to coordinate with @Volker_E to separate more of common.less
  • We agreed to not position the personal menu right now and to not touch it at all as part of this task. We agree that eventually the vertical alignment will need to change, but we agreed to descope this for now. It is a blocker for shipping. Likewise the descope technical note "On smaller widths, the personal menu in the right side of the header should not overlap the logo, it should wrap to the next line instead." has been dropped. I've added it to T240856#6073089
  • We talked about browsers without flex box and will use a mixture of display: block on .mw-logo__tagline and float left on components. The end result will not be vertically aligned but will be acceptable. We will sync later with @Jdrewniak after this task completes to see if we want to fine tune that.
  • We agreed to drop breakpoints in new Vector and simplify to one breakpoint for now. This will be noted in the patch commit.
  • We talked about the sidebar placement and realised the mock in the description is wrong. Alex will update task description with sidebar placement in VectorNeue with guidelines of the spacing between the bottom of the 54px header to the link for "Main page".

@alexhollender please add anything I've missed!

@Jdlrobson thanks for the notes above, they all look correct to me. As discussed I've updated the images in the Design section of the task description, and included an image that shows the spacing and alignment of the various elements.

Okay, patch is up and have run by the changes with Alex > https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/577682
Ideally would land with https://gerrit.wikimedia.org/r/587855 which removes the gradient, but have checked with Alex and that doesn't necessarily need to be a blocker right now to landing this patch. Reviews welcomed.

Demian added a comment.EditedApr 23 2020, 5:51 AM

Great news!

  • The header container should be 54px amd the icon in the logo should be 50px tall. Will fix this up later today.

The new height is 69px = 50px + 2*2px + 10px + 5px; down from 78px = 50px + 16px + 12px
Imo this is still too big, especially after the header will be sticky. In comparison popular sites often have a header around 50px (reddit 49px, twitter 54px, youtube 56px, etc.)
50px + 2*5px (as seen in this DEMO with sticky header and DEMO at current status) could be enough, I find that most balanced with the useful content.

  • We agreed to not position the personal menu right now and to not touch it at all as part of this task. We agree that eventually the vertical alignment will need to change, but we agreed to descope this for now. It is a blocker for shipping.

You're welcome to merge patch 585629 which implements vertical alignment.

  • We talked about browsers without flex box and will use a mixture of display: block on .mw-logo__tagline and float left on components. The end result will not be vertically aligned but will be acceptable. We will sync later with @Jdrewniak after this task completes to see if we want to fine tune that.

Thank you for making good use of volunteer contributions.

Change 585367 merged by jenkins-bot:
[mediawiki/skins/Vector@master] [modern] Add Header element and styles

https://gerrit.wikimedia.org/r/585367

@Volker_E @alexhollender There's a problem with inherited font-size in the logo:

  • Images are fixed size (defined in pixels in LocalSettings)
  • line-height and non-image Wordmark are relative to the browser's font-size.

Result with 32px (200%) browser font-size:

I don't think the small Logo size compared to the text is a problem. Although the images could be stretched, that has issues.
However, the misalignment caused by the increased line-height can be resolved by fixing the logo's font-size to a pixel value, presumably 16px.

Demian added a comment.EditedApr 30 2020, 5:23 PM

@Jdlrobson Over-exaggerated sample with 48px (300%) browser font-size:

@Jdlrobson Tip: adding the patch 585856 to your PatchDemo will configure wgLogos.

The current margin + padding solution in the header makes it unthemeable for user-styles for ex. There will be other issues with it. This is how it would look:

This is unnecessary, I've given a simpler, cleaner solution that I hope you will use.

Change 577682 merged by jenkins-bot:
[mediawiki/skins/Vector@master] [modern] A new version of Vector with a new logo

https://gerrit.wikimedia.org/r/577682

Demian added a comment.EditedApr 30 2020, 11:27 PM

@Volker_E @Jdlrobson @Niedzielski @nray To speed up progress on the DI work I've rebased my patch chain that goes all the way to the sticky header and user menu dropdown.

The first patch (585629: "[modern] Vertically align the PersonalMenu with the Logo") wraps up the questions that were left open in the recently merged Logo patch and prepares the ground for following work on the header, such as T249363.
This patch is ready for review.

The following patches take further steps towards the final goal, please see if there is something that can be used from those at this stage.

Context: last week when I spoke with @Jdlrobson we discussed ensuring that the logo still looks good on browsers that don't support Flexbox. As a result some CSS was updated, however I think now we've lost the vertical centering of the tagline.

beta (incorrect)centered (correct)
LTR
RTL

I think one solution would be:

  • make the .mw-logo-container element a flexbox with the following CSS
display: flex;
flex-direction: column;
align-items: center;
align-self: center;
  • add margin: 0 auto to the tagline, and move the margin-top: 5px to be a margin-bottom: 5px on the wordmark (this is for browsers that don't support Flexbox)

@alexhollender - should we open a new task to track this?

I think now we've lost the vertical centering of the tagline.

The horizontal centering...
It was removed in PS30 on 30 Apr:
https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/577682/29..30/resources/skins.vector.styles/Logo.less#b29

@Volker_E wrote:

In a conversation with Jon Robson, it doesn't make sense in the current layout to align wordmark and tagline center, it's harder to read and not expected from current design requirements in agreement with Alex Hollender.
Therefore removing auto 0 again and going back to margin-top only.

A bit confusing...

I think one solution would be:

  • make the .mw-logo-container element a flexbox with the following CSS
display: flex;
flex-direction: column;
align-items: center;
align-self: center;

This was explored previously, margin: 0 auto is the fallback anyway for non-flex browsers, making the use of flexbox unnecessary.

  • add margin: 0 auto to the tagline, and move the margin-top: 5px to be a margin-bottom: 5px on the wordmark (this is for browsers that don't support Flexbox)

This works for all supported browsers.

The wordmark element is always present, but the tagline might be missing. In the latter case it's better to remove the 5px margin too, thus imo the margin-top: 5px should be on the tagline. Moving it is not necessary, just combine the two rules: margin: 5px auto 0;
or write the 2 rules in order, which is more readable:

margin: 0 auto;
margin-top: 5px;
Gilles added a subscriber: Gilles.May 7 2020, 9:04 AM

It seems like the current PNG globe icon I'm seeing on beta could be significantly smaller in terms of file weight. With ImageOptim, lossless gains are around 15% and lossy at 57%. Even with the lossy version you would need a microscope to see differences.

It seems like the current PNG globe icon I'm seeing on beta could be significantly smaller in terms of file weight. With ImageOptim, lossless gains are around 15% and lossy at 57%. Even with the lossy version you would need a microscope to see differences.

T245362 implies that [some] people use ImageOptim for our logos. If specific production logos are not optimized there should be specific tickets IMHO.

Gilles added a comment.EditedMay 7 2020, 10:28 AM

I'm talking about the new version of the logo introduced by this prototype, in case that wasn't clear. I think the lossy version ImageOptim generates needs some quality review by designers working on this project.

I'm happy to do the same to all the production logos in /static/ as part of a separate task (the gains are about the same on the production enwiki logo as the gains seen for this prototype logo), but I think visual quality should be vetted by someone who has responsibilities in that area, if we're going to consider the lossy version.

Filed T252108: Optimise production wiki logos if a designer wants to review the lossy version there, that would be much appreciated.

For the sake of easy comparison and to stay within the context of this task:

Current prototype logo:

ImageOptim version, lossless mode (15% smaller file weight than original):

ImageOptim version, lossy mode (57% smaller file weight than original):

Filed T252108: Optimise production wiki logos if a designer wants to review the lossy version there, that would be much appreciated.

Both the lossless and lossy versions look great to me. Thanks so much for investigating this @Gilles.

I've increased the scope of T252143 to take this into account..

Looks great. I've checked various headers across projects.

ovasileva closed this task as Resolved.May 26 2020, 4:14 PM
ovasileva updated the task description. (Show Details)

Resolving. Documentation will be up on mediawiki by the end of the week.

@alexhollender The logo size is not relative to the font-size, resulting in disproportionate logo sizes with big browser font-size (150%, 200%). Not many, but there are some users who have that setting for accessibility reasons. 200% looks like:

This is related to the wordmark vertical misalignment at increased font-size, see previous comment on this topic: T246170#6095802

Change 585856 abandoned by Aron Manning:
[DNM-POC] [for Patchdemo] Define new header $wgLogos from T246170

Reason:
Patchdemo has its own logo now and this stopped working.

https://gerrit.wikimedia.org/r/585856

@alexhollender The logo size is not relative to the font-size, resulting in disproportionate logo sizes with big browser font-size (150%, 200%). Not many, but there are some users who have that setting for accessibility reasons.

Thanks for pointing this out. I don't think it needs to be addressed urgently but is good to know about. It seems that currently the logo doesn't respond to browser font-size settings:

cc @Volker_E for future consideration

Change 585856 restored by Aron Manning:
[DNM-POC] [for Patchdemo] Define new header $wgLogos from T246170

https://gerrit.wikimedia.org/r/585856