In Progress
Status Update
Comments
fr...@google.com <fr...@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.
br...@google.com <br...@google.com> #3
Thanks Ben :)
fr...@google.com <fr...@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
fr...@google.com <fr...@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
is...@google.com <is...@google.com> #6
Edits were made to reflect the following in Monorail: auto-CCs.
Description
2. Navigate to local page
Expected:
Fonts and other static assets are loaded
Actual:
Fonts and other static assets are missing
This is a regression when migrating from the Go server to Web Dev Server.