Skip to content
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

Feature Request: Add Setting for object-fit Cover vs. Contain #15

Open
jeremy-farrance opened this issue Mar 18, 2022 · 3 comments
Open

Comments

@jeremy-farrance
Copy link

The way SwiperJS is used it assumes photos, so the CSS (.app-sw... picture img) applies object-fit: cover. If you want to use logos or other types of images where cover doesn't make sense, the simple fix is object-fit: contain.

In the next version it would be an awesome setting to add at both the default, app/page/module levels.

The problem demonstrated with COVER

image

And fixed with CONTAIN

image

@iJungleboy
Copy link
Contributor

Thanks for this.

I'm not quite sure yet how we should address this, as the problem doesn't just occur in the swiper but everywhere.
I guess a core problem is that it could affect any picture - even standard content images which get mis-cropped.
I'll try to think of something more generic.

Here's basically what I think we need

  1. A mechanism to determine how the pic should be cropped - including this fit-option etc.
  2. Ideally it has some kind of possibility to say "don't use default, but..."
  3. Then it should have options like the contains, but also something like "this is the primary area of the image, all this should be visible" - which would adjust where the cropping happens
  4. Ideally at various levels - the most specific would have the highest priority
    1. the field level - on the field where you select the image
    2. maybe on the image-level (image metadata?)
    3. Maybe at the app-instance level (module)
    4. Maybe at the app level
    5. I believe site / global doesn't make sense
  5. This could affect the image resizer url params...
  6. ...but also the img tag in the way you describe above

@jeremy-farrance your thoughts?

@jeremy-farrance
Copy link
Author

jeremy-farrance commented Mar 19, 2022

I don't think its a setting where you would want or need to change it on a per-pic basis. I can't think of a use case for that at the moment, though I am sure there are some. There is a fairly common use-case for displaying logos (and diagram components, etc) and those images usually are on a white or transparent background. Its all or nothing at the module level. Here is an incomplete demo I did on this last year, CSS only, no frameworks, and not related to Swiper.

You are introducing a lot of great additional functionality with your thoughts above though, and most of these we've needed in the past, some we've built to a limited extent, but they mostly seem separate. I am glad to see you've encountered MANY situations where slicing and dicing images got complicated. :)

So, some quick thoughts.

  1. We've solved a lot of scenarios for this, but the edge cases get endless quickly. I find it hard to determine which possible features would be the most common. We do usually implement an "anchor to edge" (using ?anchor= URL params) and that is definitely common and useful in a lot of scenarios. Instead of edge, we've experimented also with setting a focal point (similar to 3 below) on the image. A lot of the solutions here (and others below) combine URL params to "reshape" the image as well as CSS settings. In our experiments, we've found that its important to keep the two separate (crop, then CSS) and let the user choose what works best (by making the settings easy to change).
  2. Yes, definitely useful. A bool that just says "use module/inherited default" vs custom settings.
  3. It sound like you are saying: let the user specify a rectangular region that must fully make it in to the final visible area of the image. Hard to imagine being able to deliver on that, which it why I usually relax the idea a little and simply use a "focal point" or area to focus on. E.g. use rule of thirds and allow the 4 intersection points to be specified as the center of the final image? (which of course in some cases means your final image is no longer the photographer's framed rule of thirds masterpiece).
  4. Yes, but I do believe all levels make sense. We've got sites with 3 different swiper uses - banner slider, inside page slide show, and logo batch (like the link above). Its always nice (time saving) when the out of the box defaults just work in at least 1 scenario.
  5. Yes, almost always for everything we've done to date.
  6. Yes, and to show off a practical example, to fix the example (screenshots) above, we are able to do it with this simple addition in bs4/Helper.cs (in a custom View with renamed _swiper.cshtml and Helper.cs):

before:

    pictureTag.Add(
      Tag.Img().Src(Link.Image(imgUrlOrig, resizeSettings)).Alt(title)
    );

after:

    pictureTag.Add(
      Tag.Img().Src(Link.Image(imgUrlOrig, resizeSettings)).Alt(title).Style("object-fit:contain;")
    );

@jeremy-farrance
Copy link
Author

One other thing I almost forgot to point out - and this is just my humble opinion and not well thought through - but, in regards to 1 and 3 (and others) I have the perception that instead of building (too) many settings/options with complex output scenarios, the MUCH easier approach would be to include an existing functional library (.NET or JS?) that instead lets the user manually crop and resave the image (optionally keeping the original?). I realize the technical debt of introducing a dependency, but I definitely feel with would be a much better solution to the in-the-moment case where the image ends up displaying a way that makes the user want to change it (even when they don't know how, which results in a support ticket or email).

Anyhow, I hope that idea is on your radar and that you see how many exponential edge-cases it could solve for (which would be hard to account for in settings and code). Cheers!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants