-
Notifications
You must be signed in to change notification settings - Fork 598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
imagebox: add a "cover" fit policy #3811
base: master
Are you sure you want to change the base?
Conversation
This fit policy emulates the behaviour of CSS' `background-size: cover` Signed-off-by: delta <darkussdelta@gmail.com>
Hello, Can you edit the following 2 files and post a screenshot of the result? This should regenerate the 2 images you see in the doc. https://github.com/awesomeWM/awesome/blob/master/tests/examples/wibox/widget/imagebox/horizontal_fit_policy.lua |
Signed-off-by: delta <darkussdelta@gmail.com>
In that case, there's probably some interaction with |
Codecov Report
@@ Coverage Diff @@
## master #3811 +/- ##
=======================================
Coverage 90.99% 91.00%
=======================================
Files 900 900
Lines 57506 57512 +6
=======================================
+ Hits 52329 52336 +7
+ Misses 5177 5176 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
it does appear to. |
On a general level, I'll want to throw in that For the change itself, the result in the latest screenshot doesn't match what I would expect from the description and name of the option. |
i'll just throw in how it's described in css spec, so everyone would have the same understanding of how it expected to work:
https://w3c.github.io/csswg-drafts/css-images/#the-object-fit |
If we do go with the CSS spec, then the smallest rectangle is where the above screenshot doesn't comply with the constraint:
|
i specifically posted the pics in case if anyone would have troubles with english |
I meant the image from delta that demonstrates the proposed change, not the one from MDN. |
yup - that's the intention, to make it visible side-by-side i think the problem that both W and H are computed as max, whilst it should be done only for one of the aspects, and other computed from it |
This is the same as #3554 and will probably have the same issue as #3547 (comment). Shame that awesomwm is essentially a dead project. |
@alanswanson your PR is missing doc examples and tests as in this one - mb you could combine effort to make one good PR from two 😺 |
@actionless As mentioned in #3547 (comment), they were waiting for the code changes to be reviewed before spending time on docs. |
that's what i did in the comment above - that information should be enough to make one of PRs merge-able |
This fit policy emulates the behaviour of CSS'
background-size: cover