2sic / app-swiper

DNN / 2sxc Slider App using the popular Swiper JS
MIT License
0 stars 1 forks source link

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

Open jeremy-farrance opened 2 years ago

jeremy-farrance commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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!!