Closed mpourismaiel closed 4 years ago
Failing test is fixed in #801, It's unrelated to this PR.
Instead of having the background on the overlay it should be on the portfolio item itself I think. So mainly usable with transparent images or logos such as the github icon.
What do you think? Not sure on the usage of overlaying the background image on hover.
So I misunderstood the issue. And I'm afraid I don't understand your comment. Would you please elaborate? @stp-ip
There are probably 2 different interpretations and usage modes, which I probably miscommunicated.
So one is to have the ability to overlay images. Meaning one can have a background image and a transparent image/logo overlayed.
The other one is what you created, which is have the main image and overlay another image on top. Small note here. It should probably also overlay the icon.
Thinking this through I do like both approaches and it probably depends on what the main outcome is that we want. In my mind as far as I can recall I was thinking to have the ability to have a background image overlayed by the icon or a transparent image, but having the ability to hover and it overlaying another image could be interesting.
So it could showcase a person and then overlay a company logo or it could showcase a logo and overlay a screenshot or vice versa.
What do you personally think might be more beneficial. In theory we could create both, but that's just another customization again. It would lead to:
To be honest I couldn't understand the need of overlay but added it anyway :D I like the background idea much more and remove the overlay if you agree.
Should have made the intention more explicit. OK with using just background :)
So, how do we limit the size of the image? Does it ever cover the entire item?
I would say both the image as well as the background should be the same size. The effect to overlay would only work with a transparent image or an icon. What do you think?
Limiting the size might be doable similar to what we did with the items fragment?
Adding opacity: .7
to the image and adding a background for first item results in the screenshot above, also note the third item which has a background and an icon. Also first item's background is a bird and the image is of a cat.
Is this a good implementation? Also, should the exact same effect be implemented in the modal?
I wouldn't add any opacity manually. If users want transparency they can add this into their image.
As this is mainly a background, I would have said it shouldn't show up in the modal, but I see your point and I think you are right and adding this without opacity there as well makes sense.
@stp-ip Removed opacity and added background to the modal.
Any reason not to showcase this within our portfolio fragment? Similar to what you showed with it being the background to the github icon for example?
@stp-ip I forgot to add it to the PR.
@stp-ip I added the example before my comment. I think you might have missed it. Or should I do something else?
Missed it all good and merged :)
What this PR does / why we need it: Add support for background image in overlays of portfolio fragment
Which issue this PR fixes: fixes #227
Release note: