header icons now sit more vertically centered

[michaelchadwick]

Fixes ilios/ilios#5700

[stopfstedt]

this throws things out of alignment for me on Chromium and FF in .

Chromium, fullscreen ❌

vertical spacing around navbar buttons is now uneven.

old

new

Chromium, mobile ✔️

minor difference in spacing, but looks good.

old

new

Firefox, fullscreen ❌

vertical spacing around navbar buttons is now uneven.

old

new

Firefox, mobile ❌

lack of bottom vertical spacing in header.

old

new

[stopfstedt]

these changes throw the layout off in FF and Chrome.

[michaelchadwick]

@stopfstedt thank you for the screenshots. Will look into updating further.

[michaelchadwick]

@stopfstedt OK, I did a ton more work trying to make this work on both Chrome/Firefox. The spacing is not totally uniform on each due to what I can only assume is rendering engine differences, but I think it's better than it was.

[stopfstedt]

@michaelchadwick - thanks for making these changes. much improved in mobile and on the login screen on all screen-sizes.

however, the vertical spacing around header elements is now still uneven on fullscreen.

now:

before:

at least it's consistent now between Chrome and FF.

i'm willing to accept this as a trade-off, but leaving it up to @jrjohnson and/or @dartajax to make a decision here.

[michaelchadwick]

@dartajax Belay that for a bit. I have another go at this in me...

[michaelchadwick]

@stopfstedt Looks like this file may be the real culprit: https://github.com/ilios/frontend/blob/master/packages/ilios-common/app/styles/ilios-common/fonts.scss

Once you get to 1200px, it switches to a calc() value which causes the font-size in the buttons to slowly increase as the viewport does. At 1920px, it switches back to a static size, but either way the buttons get too big for the header and we lose the vertical alignment.

I know we disallow making CSS font-size changes, but we may need to make an exception for the header buttons.

[michaelchadwick]

@stopfstedt I did some more changes and made it even better...?

[dartajax]

Just to get this thing going, I added the READY TO MERGE tag - doesn't necessarily mean I am going to immediately merge it but let's see if the tests pass, eh?

[dartajax]

looks good to me

[stopfstedt]

LGTM.

[michaelchadwick]

@dartajax We are good to go code-wise, but Percy is requesting a mess of visual changes for review.