FIX Vertically centre CMS logo icon in menu

[purplespider]

Description

The Silverstripe CMS logo icon in the left menu is slightly lower than it should be. This is a regression introduced in 6.2.0-beta1 by #1988, which added vertical-align: middle to the .font-icon-silverstripe-cms:before pseudo-element inside .cms-sitename__link, after wrapping it in a span to hide the icon from screen readers.

Manual testing steps

1. Log in to the CMS
2. Collapse the left menu by clicking the hamburger icon
3. Observe the Silverstripe CMS logo icon — it should be vertically centred within its blue square

Issues

• CMS logo icon not vertically centred in collapsed menu (6.2 regression) #2121

Pull request checklist

• The target branch is correct
  • See picking the right version
• All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
  • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes
    to be reverted, and submitted as a separate PR.
• The commit messages follow our commit message guidelines
• The PR follows our contribution guidelines
• Code changes follow our coding conventions
• This change is covered with tests (or tests aren't necessary for this change)
• Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
• CI is green

[GuySartorelli]

Just to validate before I merge this, is "before" in your gif "before this PR" or "before the change in #1988 "?

I ask because I thought it meant "before this PR" (since you said "The Silverstripe CMS logo icon in the left menu is slightly lower than it should be."), but I notice the icon is slightly lower when I apply this PR compared to the beta.

[purplespider]

Odd. It is "before this PR" as you thought, i.e. in 6.2.0-beta1 and when I apply the fix to 6.2.0-beta1, the icon does move up to become centred, in the exact same position as 6.1. I've checked in Chrome and Safari, and I've also manually tested changing vertical-align: middle; to display: block; in dev tools and it behaves as expected, i.e. moves up slightly.

[GuySartorelli]

I just compared again to confirm - here they are side-by-side, left is without this PR and right is with this PR. The difference is almost imperceptible even with reference lines, but the logo on the left (before this PR) is just barely higher than on the right (with this PR)

Those screenshots were taken in Firefox. It looks like in Chrome I get the opposite effect - the logo goes up after the PR. So just some browser differences, I guess.

[purplespider]

That is very odd. I'm seeing the original issue in Firefox, Chrome, and Safari. So doesn't just appear to be down to browser difference. It's out of alignment more than your screenshot shows too.

Forgetting about the PR for a minute, I can confirm the following:

• On 6.1.0 the icon is visibly centred in Vivaldi (Chromium), Safari and Firefox.
• On 6.2.0-beta1 the icon is visibly too low and not vertically centred.

This graphic shows: Left to right: 6.1.0 on Vivaldi (Chromium), Safari and Firefox, and then 6.2.0-beta1 on Vivaldi (Chromium), Safari and Firefox:

Note how it's visibly lower across all 3 browsers in 6.2.0-beta1, and no longer aligned nicely with the "Pages" title. All clean Silverstripe installs.

---

For the 6.2.0-beta1 sites (the last 3), if I then go into dev tools and change vertical-align: middle; to display: block;:

The icons now line up perfectly to those in 6.1.0 for all 3 browsers:

Hopefully, this helps you to reproduce it.

[GuySartorelli]

I'm not trying to reproduce the bug, just the results after this PR. In my firefox browser, this PR makes the logo go down, but you say it should go up. That's all that's holding me up from merging.

[purplespider]

Sorry, Guy, my point was that maybe you're not seeing the bug in the first place, which would explain why you're not seeing the expected effects of the PR.

Do you see the bug detailed in my last screenshots when you compare 6.1.0 to 6.2.0-beta1?