CLDR-17947 site: pin image width, win fix #4054
Conversation
|
✨ deployed to https://b1085791.cldr.pages.dev |
There was a problem hiding this comment.
Small enough change and I know the context so I'm happy to accept.
For future PRs -- when you post PRs can you include screenshots and links to the before/after? It will help future CLDR developers have context for the fix.
From slack here's an example page that is helped by this fix: https://cldr.unicode.org/index/language-support-levels
| @@ -180,3 +180,8 @@ header#atViewHeader { | |||
| div > header { | |||
| display: none !important; | |||
| } | |||
|
|
|||
| img { | |||
| max-width: 100%; | |||
There was a problem hiding this comment.
Sounds like we should use svh & svw
There was a problem hiding this comment.
There was a problem hiding this comment.
@macchiati what are you seeing instead? When I used dvh/dvw from your original proposal, the scale didn't work properly- it was far too wide still.
| @@ -180,3 +180,8 @@ header#atViewHeader { | |||
| div > header { | |||
| display: none !important; | |||
| } | |||
|
|
|||
| img { | |||
| max-width: 100%; | |||
There was a problem hiding this comment.
|
… On Mon, Sep 16, 2024 at 12:21 PM Conrad Nied ***@***.***> wrote:
***@***.**** approved this pull request.
Small enough change and I know the context so I'm happy to accept.
For future PRs -- when you post PRs can you include screenshots and links
to the before/after? It will help future CLDR developers have context for
the fix.
From slack here's an example page that is helped by this fix:
https://cldr.unicode.org/index/language-support-levels
—
Reply to this email directly, view it on GitHub
<#4054 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACJLEMCZ37ZQNKLHJNL7RELZW4VUDAVCNFSM6AAAAABOJ5NEE6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMBXGU4DGNRRGI>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
added to the top |
img {
max-width: 100svw;
max-height: 90svh;
}gives this which looks wrong to me (still a huge image, twice as wide as the page):
compare to the current PR version: https://b1085791.cldr.pages.dev/index/language-support-levels#language-selection-ui img {
max-width: 100%;
max-height: 90dvh;
}
Do you prefer the former? (the '90' is so that a super tall image still leaves some text above or below the image, so you can see there's something to scroll to.) |
|
That is very strange. Let's go with what you have; we can check out alternatives later. |
|
Just tried it out; looks great. |
CLDR-17947
also:
fix builder for windows pathnames
This PR completes the ticket.
Testing
ALLOW_MANY_COMMITS=true