Fixed
Status Update
Comments
br...@google.com <br...@google.com> #2
Ok, here is a fix:
422524: Fix vertical alignment of header icon and text |
I have spent enough time to fully understand what is going on, so I could as well fix the issue. :-) What convinced me is that even all the current installations (e.g. chromium-review) have improved. So this was not just a feature request for using other icon sizes, but this was an alignment bug everywhere, even though it mostly matters just 1 or 2 pixels.
ka...@linaro.org <ka...@linaro.org> #3
Thanks Ben :)
ap...@google.com <ap...@google.com> #4
Project: gerrit
Branch: master
commit 06cfcb6ef54d78d7e68dfc0f17a15d30d6aae414
Author: Ben Rohlfs <brohlfs@google.com>
Date: Fri Apr 26 11:28:12 2024
Fix vertical alignment of header icon and text
There is generally a slight misalignment of icon and text as can be
seen in this screenshot:https://imgur.com/a/tDUHGk0
It was also noted in the referenced bug that increasing the icon height
is impossible, because then the alignment gets totally messed up.
3 problems contribute to this misalignment:
1. `.titleText` element being a span. This generally makes alignment of
icons next to text very tricky. Let's use a flex element instead.
2. `vertical-align: text-bottom` for the icon is a very unusual choice
for the vertical-align property and does not really make a lot of
sense. Let's just use the `align-items: center` style for flex
elements, see above.
3. `.bigTitle` has a custom font size, but not a custom line-height.
That is generally problematic as can be seen here: The element will
get a height of 20px as an inline element, but contains text with a
24.5px font. No surprise that we see misalignment. Let's make sure
that the line-height and thus the height of the text element has a
proper value. 1.2*font-size is what browsers would normally use.
This was checked to behave well on gerrit-review, android-review and
chromium-review.
Release-Notes: skip
Bug: Issue 337076005
Change-Id: Ia09bbe92158f7b4b1d6b88e599e0635fa9b15d5e
M polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
M polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.ts
https://gerrit-review.googlesource.com/422524
Branch: master
commit 06cfcb6ef54d78d7e68dfc0f17a15d30d6aae414
Author: Ben Rohlfs <brohlfs@google.com>
Date: Fri Apr 26 11:28:12 2024
Fix vertical alignment of header icon and text
There is generally a slight misalignment of icon and text as can be
seen in this screenshot:
It was also noted in the referenced bug that increasing the icon height
is impossible, because then the alignment gets totally messed up.
3 problems contribute to this misalignment:
1. `.titleText` element being a span. This generally makes alignment of
icons next to text very tricky. Let's use a flex element instead.
2. `vertical-align: text-bottom` for the icon is a very unusual choice
for the vertical-align property and does not really make a lot of
sense. Let's just use the `align-items: center` style for flex
elements, see above.
3. `.bigTitle` has a custom font size, but not a custom line-height.
That is generally problematic as can be seen here: The element will
get a height of 20px as an inline element, but contains text with a
24.5px font. No surprise that we see misalignment. Let's make sure
that the line-height and thus the height of the text element has a
proper value. 1.2*font-size is what browsers would normally use.
This was checked to behave well on gerrit-review, android-review and
chromium-review.
Release-Notes: skip
Bug:
Change-Id: Ia09bbe92158f7b4b1d6b88e599e0635fa9b15d5e
M polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
M polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.ts
ap...@google.com <ap...@google.com> #5
Project: gerrit
Branch: stable-3.8
commit 518840c5b86419c19e4fa2616e273a25998fad9b
Author: Ben Rohlfs <brohlfs@google.com>
Date: Fri Apr 26 11:28:12 2024
Fix vertical alignment of header icon and text
There is generally a slight misalignment of icon and text as can be
seen in this screenshot:https://imgur.com/a/tDUHGk0
It was also noted in the referenced bug that increasing the icon height
is impossible, because then the alignment gets totally messed up.
3 problems contribute to this misalignment:
1. `.titleText` element being a span. This generally makes alignment of
icons next to text very tricky. Let's use a flex element instead.
2. `vertical-align: text-bottom` for the icon is a very unusual choice
for the vertical-align property and does not really make a lot of
sense. Let's just use the `align-items: center` style for flex
elements, see above.
3. `.bigTitle` has a custom font size, but not a custom line-height.
That is generally problematic as can be seen here: The element will
get a height of 20px as an inline element, but contains text with a
24.5px font. No surprise that we see misalignment. Let's make sure
that the line-height and thus the height of the text element has a
proper value. 1.2*font-size is what browsers would normally use.
This was checked to behave well on gerrit-review, android-review and
chromium-review.
Release-Notes: skip
Bug: Issue 337076005
Change-Id: Ia09bbe92158f7b4b1d6b88e599e0635fa9b15d5e
(cherry picked from commit 06cfcb6ef54d78d7e68dfc0f17a15d30d6aae414)
M polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
M polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.ts
https://gerrit-review.googlesource.com/425217
Branch: stable-3.8
commit 518840c5b86419c19e4fa2616e273a25998fad9b
Author: Ben Rohlfs <brohlfs@google.com>
Date: Fri Apr 26 11:28:12 2024
Fix vertical alignment of header icon and text
There is generally a slight misalignment of icon and text as can be
seen in this screenshot:
It was also noted in the referenced bug that increasing the icon height
is impossible, because then the alignment gets totally messed up.
3 problems contribute to this misalignment:
1. `.titleText` element being a span. This generally makes alignment of
icons next to text very tricky. Let's use a flex element instead.
2. `vertical-align: text-bottom` for the icon is a very unusual choice
for the vertical-align property and does not really make a lot of
sense. Let's just use the `align-items: center` style for flex
elements, see above.
3. `.bigTitle` has a custom font size, but not a custom line-height.
That is generally problematic as can be seen here: The element will
get a height of 20px as an inline element, but contains text with a
24.5px font. No surprise that we see misalignment. Let's make sure
that the line-height and thus the height of the text element has a
proper value. 1.2*font-size is what browsers would normally use.
This was checked to behave well on gerrit-review, android-review and
chromium-review.
Release-Notes: skip
Bug:
Change-Id: Ia09bbe92158f7b4b1d6b88e599e0635fa9b15d5e
(cherry picked from commit 06cfcb6ef54d78d7e68dfc0f17a15d30d6aae414)
M polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
M polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.ts
Description
What steps will reproduce the problem?
1. Install a JS plugin to add an icon in the header.
What is the expected output?
The icon is displayed without impacting alignment of other components in the header
What do you see instead?
The icon causes the header-title-content to no longer be centered. Try increasing the icon size if you don't see the issue.
What is the output of the JS console log (if applicable)?
What is the performance record (see
Please provide any additional information below.
Possible (but not acceptable) fix to the issue: