UI changes to support current responsive image standards

I talked with Marc on IRC a bit about the *.sizes stuff and read a bunch of his provided background info on the picture element etc. which was really enlightening. (http://blog.cloudfour.com/dont-use-picture-most-of-the-time/, https://dev.opera.com/articles/native-responsive-images/)

My take is that *.sizes.yml is beta-safe, because it doesn't break any APIs, only adds them, and it's necessary to complete the D8 features of Mobile/HTML5, so not really introducing "new" functionality in that respect.

I think providing 2 image formatters might be easier to solve this problem, the first will be using a picture mapping allowing you to combine any breakpoint with sizes and/or image styles, the second - not available yet - is a formatter that allows you to enter (or select) sizes and a list of image styles.

FYI - OT: We implemented and 'image with sizes' display formatter, but the output is still using the picture tag to avoid that browsers will prefetch the src of the img tag.

That's true. Site builders are most likely going to want control over this.

I just want to be clear on terminology: are you proposing two image effects (which could then be combined with others such as scale, crop, and selected from the "Manage display" screen) or are you proposing two image formatters (of which the current one just lets you select different image styles). I'm a bit confused on how this would work as image formatters, at least without confusing people really badly and requiring to potentially repeat entry a bunch of times + difficult to keep the sizes synchronized across different content/entity types.

#6 In #4 I was talking about 2 formatters, 'responsive images' and 'image with sizes', we need better names but that's another discussion.

If we are able to pick meaning full names, I think it might work, most other fields do have multiple formatters as well.

Keeping sizes synchronized is a different discussion: is this needed and if so how can we solve it.

There's another use case discovered today, re-use the same breakpoint multiple times when creating a mapping, this will be needed if you want to output different MIME types for the same breakpoint. [edge case]

Hm. Let's drill into a use case more to help flesh this out.

Use case: I'm trying to migrate an e-commerce site like http://www.thinkgeek.com/ to Drupal.

I'm going to need various content types:
- Apparel
- Toy
- Gadget
- Office Supply
- Outdoors Thing
- etc.

Each of those are going to have an image field "Product image."

In a listing like http://www.thinkgeek.com/interests/gameofthrones/?icpg=HP_BrandLogos_Top... I'm going to want all of the product images (across content types) to be of a similar style. This means scaled + cropped to a square, which I can make into an Image style called "Scropped" and select from Manage displays, Views, etc. I'm also going to want them to be responsive, so if I go to that page on a cell phone I don't have a scrollbar.

How do we accomplish this if these are image formatters which would then (I think) turn your choice to "do you want them square or do you want them responsive" which is not a choice most people would want to make.

Because if I could edit "Scropped" and select a "Generate image sizes for smaller screens" (or whatever) effect, and add my sizes exactly once, then everywhere I use the "Scropped" image style (sidebar blocks, search results, etc.) would all be the same. I don't understand how to achieve the same thing if responsive is a "Generate image sizes for smaller screens" formatter instead. I would need to re-enter my sizes every single instance in which I use that formatter. No?

Discussed with @webchick and @mdrummond, IRC log for now

|21:23:55      attiks | .
|21:24:27      attiks | webchick: have a sec for https://www.drupal.org/node/2334387
|21:24:29   Druplicon | https://www.drupal.org/node/2334387 => UI changes to support current responsive image standards #2334387: UI changes to support current responsive image standards => 10 comments, 4 IRC mentions
|21:24:40    webchick | attiks: yeah, was hoping you'd join, easier to hash out in person I think. :)
|21:24:57      attiks | webchick: with the new formatter you'll have 2 options
|21:25:07      attiks | use a mapping and inside the mapping you define the sizes once
|21:25:30      attiks | webchick: you then can use it in multiple places, same as the mapping right now
|21:25:51    webchick | BUt correct me if I'm wrong… we don't have a "Formatter UI" like we do an "Image Styles UI" so you couldn't as a site builder create this.
|21:25:56      attiks | webchick: or use the new formatter: enter your sizes on the settings of the formatter and select a bunch of image styles
|21:26:00    webchick | You could programmatically define a "Product image" with your sizes?
|21:26:09    webchick | and then N other formaters for other types of iamge displays?
|21:26:18      attiks | webchick: yes
|21:26:23    webchick | So.. that sucks. :)
|21:26:30    webchick | Why can we not make it an image effect again?
|21:26:36    webchick | It's just a transformation, is it not?
|21:26:43    webchick | And is there any circumstance under which we will NOT want images to be responsive?
|21:27:09      attiks | webchick: if you want to re-use it, you need the mappings, the "new" UI is for the moment part of the picture module (contrib)
|21:27:12    webchick | attiks: like what's confusing to me is we offer an image effect "resize"
|21:27:20    webchick | to me, that's the *perfect* place to consolidate all "size" related stuff
|21:27:46      attiks | webchick: so you want it auto magically
|21:27:52      attiks | webchick: unicorn style?
|21:28:01    webchick | I don't think I'm asking for that, no.
|21:28:01    webchick | One sec.
|21:28:29    webchick | I will warn you that I only barely understand this stuff due to lots and lots of patience from mdrummond, but I'm coming at this from a "site builder experience"
|                     | perspective.
│21:28:36    webchick | Because if we're going to make a UI at all, we should make a UI that makes sense.
│21:28:59   mdrummond | attiks: Hi!
│21:29:04   mdrummond | webchick: Hello!
│21:29:08    webchick | howdy :)
│21:29:08         chx | if you do that thne where's the adventure in it?
|21:29:15      attiks | mdrummond: Hi!
|21:29:16         chx | discovering a new and entirely baffling ui
|21:29:18 geerlingguy | chx++
|21:29:19 geerlingguy | :D
|21:29:19   mdrummond | attiks: Just left a new comment on https://www.drupal.org/node/2343351
|21:29:19         chx | that's a challenge
|21:29:20   Druplicon | https://www.drupal.org/node/2343351 => [Meta] Remove Picture polyfill #2343351: Make picture polyfill optional => 8 comments, 2 IRC mentions
|21:29:27    webchick | So I'm at admin/structure/types/manage/article/display
|21:29:27         chx | why take awya the challenge
|21:29:29    webchick | here's what I see:
|21:29:37 geerlingguy | We need to sustain the #drupalwtf factor for future generations
|21:29:47      attiks | chx++
|21:29:50      attiks | geerlingguy++
|21:29:56    webchick | https://dl.dropboxusercontent.com/u/10160/%21tmp/Screen%20Shot%202014-09-23%20at%2012.29.33%20PM.png
|21:30:13    webchick | I also see that at admin/structure/views, admin/structure/panels and various other places.
|21:30:25    webchick | As a site builder "Do something to manipulate an image" == Image styles
|21:30:57    webchick | And if we look at "Large (480x480)" you see
|21:31:21    webchick | https://dl.dropboxusercontent.com/u/10160/%21tmp/Screen%20Shot%202014-09-23%20at%2012.31.04%20PM.png
|21:31:29    webchick | Resize is there, scale is there.
|21:31:37    webchick | So why do we not hinge off those existing UIs?
|21:31:44    webchick | With something like:
|21:33:34      attiks | mdrummond: thanks for the follow up
|21:33:46   mdrummond | webchick: The essential challenge of responsive images is that you need to combine a resizing of the image itself with in most cases multiple media queries that need to
|                     | be synced up with the layout of a particular site.
|21:34:07    webchick | Well I don't know. :) Something like this https://dl.dropboxusercontent.com/u/10160/%21tmp/Screen%20Shot%202014-09-23%20at%2012.33.38%20PM.png
|21:34:15    webchick | execpt with whatever fields are needed to support respimg
|21:34:27   mdrummond | webchick: A media query can only query the size of the entire browser window, not the size of the image's container.
|21:34:41      attiks | webchick: let's start with the easiest case, resolution switching
|21:34:46    webchick | Yes, let's.
|21:35:03      attiks | webchick: a site builder wants to support retina displays, so he need to define 2 image styles
|21:35:19    webchick | Why two? Why not *an* image style, with two image effects?
|21:35:24      attiks | webchick: normal_screen (1024px wide) and retina_screen(2048px wide)
|21:35:24    webchick | retaina vs. not retina
|21:35:36      attiks | webchick: 1 image style will output 1 image
|21:35:40   mdrummond | webchick: So the combination of a certain set of media queries with a certain set of image sizes might work great for one particular site's layout, or more specifically
|                     | one type of image (like a product image) in one layout, but you'd need another set of rules for a different type of image in that layout much less an entirely different
|                     | site layout.
|21:35:42    webchick | Ok, right.
|21:35:48    webchick | Can we just change that? :)
|21:35:55    webchick | So one image style can generate an img with a srcset?
|21:36:07      attiks | webchick: in theory yes
|21:36:21      attiks | webchick: in reality, not sure
│21:36:40    webchick | Because the point of an image style is to barf out an img tag and srcset goes in the img tag from what I know
│21:36:47      attiks | webchick: i think it will complicate the UI of image styles/effects
│21:36:56    webchick | Yes, most likely.
│21:37:01   mdrummond | webchick: There are two kinds of srcset, which helps make this extra less confusing.
│21:37:05    webchick | attiks: Then I guess let's go at this the other way, which is
|21:37:11      attiks | webchick: true, although srcset can go in source tag as well ;-)
|21:37:16    webchick | How do I get squared+cropped images in N different screen sizes?
|21:37:29      attiks | webchick: you make N image styles
|21:37:43      attiks | webchick: you create a mapping and select all those styles
|21:37:49    webchick | But each image style only outputs one, per your previous response
|21:37:49    webchick | ah.
|21:37:57   mdrummond | webchick: One type of srcset can put out multiple copies of an image at different resolutions: 1x, 2x, 3x to handle retina images. But that assumes the image takes up
|                     | the same amount of space no matter how big the browser is. Which often won't be true.
|21:38:05    webchick | Ok, so we introduce a new top-level terminology… um… "image mappings"? (ugh, horrible name)
|21:38:15      attiks | webchick: we're good at naming things
|21:38:16    webchick | and then that becomes basically a "collection" of image styles?
|21:38:21      attiks | webchick: yep
|21:38:22   mdrummond | webchick: responsive image mappings is what we've been using.
|21:38:28    webchick | hm
|21:38:30    webchick | yeah that'd work
|21:38:32      attiks | webchick++
|21:38:40    webchick | Ok, so I need:
|21:38:48    webchick | Product image retina displays
|21:38:54    webchick | Product image small screens
|21:38:57    webchick | Product image desktop
|21:38:58    webchick | or whatever
|21:39:02    webchick | as image styles
|21:39:09    webchick | then a "Product image" responsive image mapping
|21:39:11    webchick | that collects those 3
|21:39:17    webchick | and puts in whatever fancy HTML5 gibberish around it?
|21:39:23      attiks | webchick: correct
|21:39:28    webchick | Ok.
|21:39:29   mdrummond | webchick: The other type of srcset puts a series of image files in attribute with the size of each image listed, and then based on the media queries defined in a sizes
|                     | attribute, the browser picks which of the srcset image files is the best fit.
|21:39:35    webchick | And the "Responsive image mapping" is the thing that's the formatter?
|21:39:46      attiks | webchick: that is the current formatter
|21:39:58    webchick | Oh, right. Why don't we turn picture on in standard profile again? :P
|21:40:10      attiks | webchick: no idea, I guess we shu=ould
|21:40:15      attiks | *should
|21:40:20    webchick | I think so.
|21:40:41    webchick | Literally no one on this earth is going to know to enable that on a new D8 site.
|21:40:55    webchick | Ok, maybe 3 people. :) attiks mdrummond and now me. ;D
|21:41:30   mdrummond | webchick: If we turn it on by default, we're trusting that each theme has a default set of image styles, breakpoints and/or sizes for one or more types of images.
|21:41:34      attiks | webchick: the problem is we still need to decide on the UI, because in your example we left out the breakpoints part
|21:41:48      attiks | mdrummond: themes should do it
|21:42:05      attiks | mdrummond: we also trust that they define regions, stylesheet, javascript, ...
│21:42:10    webchick | Hm. themes shipping with image styles?
│21:42:12   mdrummond | webchick: If not, then images that use the responsive image mappings will likely look weird.
│21:42:22    webchick | I mean it's totally possible for them to do that.
│21:42:34    webchick | But I think it also entails writing *.schema.yml which would be un-fun. :(
│21:42:38   mdrummond | webchick: Morten tried that once. It works if you put an image style into config install.
│21:42:54    webchick | yep, there's no technical reason they can't
|21:43:00    webchick | it's a "how much of a bar" reason
|21:43:15    webchick | but eh. if you don't have schema.yml then your shit isn't translatable. that's just a patch away from being fixed. ;)
|21:44:15   mdrummond | webchick: I think a lot of this depends on what sort of theme people are using. If somebody is using a pre-built shiny theme with a layout that people won't muck around
|                     | with much, totally feasible to set up some sensible image stuff to go along with that.
|21:44:58    webchick | Right, like Omega and whatnot will probably do that
|21:45:00   mdrummond | webchick: If somebody is using a starter theme where everybody makes their own layout, then it's up to the person making their subtheme to know how to set up responsive
|                     | images.
|21:45:28    webchick | right
|21:45:33    webchick | damn this stuff is so complicated. :(
|21:45:40    webchick | I so wish we would've been able to provide a UI for sitebuilders.
|21:45:43   mdrummond | webchick: Trying to make it not so complicated. :)
|21:45:47    webchick | I don't see any possible way someone like me will ever have responsive images
|21:45:51      attiks | webchick: Can you have a look at https://www.drupal.org/node/2330899 if you have the time, it is the last blocker of https://www.drupal.org/node/2260061
|21:45:56   Druplicon | https://www.drupal.org/node/2330899 => Add MIME type support for image styles #2330899: Allow image effects to change the MIME type + extension, add a "convert" image effect => 19 comments, 5 IRC mentions
|21:45:56   Druplicon | https://www.drupal.org/node/2260061 => Implement Picture polyfill 2.1 #2260061: Responsive image module does not support sizes/picture polyfill 2.2 => 121 comments, 10 IRC mentions
|21:46:04    webchick | attiks: still not rtbc :(
|21:46:19    webchick | i am subscribed tho!
|21:46:37    webchick | mdrummond: ok so let me see if I kind of have this.
|21:46:42    webchick | 1) Theme has to define breakpoints
|21:46:52    webchick | these define certain widths at which the design changes (gets smaller/narrower)
|21:46:59   mdrummond | attiks: I looked it over, and it looks pretty good. If you think that other item isn't a blocker, and if you think the current code is up to snuff, probably could go
|                     | RTBC.
|21:47:06    webchick | so 300px, 600px, "wide as the screen" or whatever.
|21:47:35    webchick | 2) Responsive image mappings need to provide a selection of 1) What breakpoint and 2) What image style(s)
|21:47:35      attiks | mdrummond: it should be good, played with it all day for my presentation
|21:47:57    webchick | So if you're at 300px on a retina display, you get "Product image retina" at 300px
|21:47:58   mdrummond | webchick: A theme needs to define breakpoints primarily if they want to use art direction. If they just want their images to be the right size without any cropping,
|                     | they need to define sizes more than breakpoints.
|21:48:06    webchick | Ok
|21:48:19    webchick | What's an example of a site that uses "art direction" so I could kind of visualize that?
|21:48:28    webchick | I mean I get it by definition. Just I'm a visual person. :)
|21:48:48    webchick | and most of my prior experience with responsive is just "shove this thing in the right sidebar under the content area"
|21:48:49   mdrummond | webchick: Scroll down here here and look for the cute dog. http://usecases.responsiveimages.org/
|21:49:11    webchick | Ok, I see.
|21:49:15    webchick | So in that case we'd need a UI that's like:
|21:49:17    webchick | - What screen size?
|21:49:18   mdrummond | webchick: That image of Bo is cropped more tightly for small screens.
|21:49:19    webchick | - What resolution?
|21:49:27    webchick | - What image effect (scale + crop)
|21:49:37    webchick | and/or - What alternate image?
|21:49:53      attiks | webchick: alternate image?
|21:49:53    webchick | attiks: will you be in AMS by chance?
│21:50:04    webchick | Well, could the dog swap out to a duck at a certain screen size?
│21:50:04      attiks | webchick: I'm even going to be on stage ;-)
│21:50:08    webchick | woohoo. :)
│21:50:10   mdrummond | webchick: No.
│21:50:19    webchick | No? I thought you had to provide multiple files
|21:50:25      attiks | webchick: nope duck is not allowed
|21:50:32    webchick | in a picture element
|21:50:37   mdrummond | webchick: Art direction should just be for cropping things. Switching out the entire image is a no-no. Bad for accessibility, for one thing.
|21:50:41      attiks | webchick: the input is always the same file
|21:50:43    webchick | so what's to stop me from showing a duck to nokia users? :D
|21:50:45    webchick | ah, ok
|21:50:47     ksenzee | webchick: NO DUCKS
|21:50:52      attiks | ksenzee++
|21:50:54    webchick | DAMN IT. I LOVE DUCKS.
|21:51:07      attiks | webchick: I'll bring one to AMS for you
|21:51:16    webchick | like how do you NOT swap out http://cdn.cutestpaw.com/wp-content/uploads/2013/04/l-Baby-ducks.jpg at 120px?
|21:51:20    webchick | I ask you
|21:51:27     ksenzee | ahahaha
|21:51:27    webchick | Why even build a website if you can't have baby ducks?
|21:51:30    webchick | anyway. ;)
|21:51:36    webchick | attiks: when's your session?
|21:51:39    webchick | It sounds like I should go. :D
|21:51:42   mdrummond | webchick: I have a good idea in my head for how to get the UI for the responsive image mapping working to handle both the breakpoint and the sizes use cases.
|21:51:50      attiks | webchick: Thue 5PM
|21:52:23    webchick | mdrummond: oh cool. Even if you could draw it on a napkin and take a picture on your phone it might be good to get the UX talk started early. That usually takes a few
|                     | weeks to sort out
|21:52:24   mdrummond | webchick: Not quite sure how to make that image in my head a reality. Might just need to build it.
|21:52:39   mdrummond | webchick: I could do some paper sketches, sure.
|21:52:42    webchick | and I think a lot of UX people will be AMS
|21:52:47    webchick | so might be able to provide feedback there
|21:53:28   mdrummond | webchick: I think the thing we need to keep in mind is that what we're aiming for right now is not Magical Unicorn Layout UI, but make it fairly straightforward to
|                     | associate image styles with breakpoints/sizes.
|21:53:42    webchick | I'm 500% on board with that.
|21:53:54   mdrummond | webchick: If we can get Magical Unicorns later, I'm all for that. I just want something that makes this actually function right now.
|21:54:03    webchick | I just want to make sure the labels are enough for people who literally think of images as <img src=".."> at best can grok what they're doing in that UI
|21:54:14    webchick | Magical Unicorns can be 8.1.x ;)
|21:55:45   mdrummond | webchick: The other challenge honestly is balancing Defining Things in the UI versus Defining Things in code-based things like YML That Is Closer to the Sass Files
|                     | Where Most Front End Deveopers That Will Understand This Stuff and Use It Will Be Able to Best Make Use of It
|21:55:56    webchick | LOL
|21:56:00    webchick | That was … quite the acronym. ;)
|21:56:07    webchick | UI is for site builders. Not front-enders.
|21:56:07   mdrummond | webchick: ;)
|21:56:10    webchick | YAML is for front-enders.
|21:56:22    webchick | If we're going to provide a UI, it needs to be for site builders, and we should not assume an understanding of the underlying HTML.
│21:57:16   mdrummond | webchick: What I'd like to aim for is to get a sizes.yml file similar to breakpoints.yml so that those layout definitions can be close to where they get defined in the
│                     | sass/css.
│21:57:38    webchick | right. that *seems* to be closer to the status quo *and* lets us postpone the UI decision
|21:57:45    webchick | and discussion
|21:57:55   mdrummond | webchick: Then the associations of image styles with breakpoints/sizes is what happens in the UI.
|21:57:56      attiks | mdrummond: i'm liking the idea of sizes more and more
|21:57:58    webchick | as long as you think front-enders would get "sizes" as a trigger
|21:58:14   mdrummond | webchick: Yes. Lots of education going on about this now.
|21:58:17    webchick | and that they're referroing to image sizes not sizes of… layouts, etc.
|21:58:28    webchick | "sizes" is a fairly ambiguous word.
|21:58:49      attiks | webchick: sizes refers to viewport size, but is bad named
|21:58:58    webchick | like as a non-front-ender who had to hack together a theme, I would probably assue "sizes" dealt with layout, and I'd have no idea wgat breakpoints did so I would not
|                     | bother making one. ;)
|21:59:10   mdrummond | webchick: In my magical unicorn world, would love for sizes to get used for more than just images some day. But that's another discussion.
|21:59:34    webchick | attiks: / mdrummond then should it be *viewports.yml?
|22:00:06   mdrummond | webchick: I'll ship you a copy of my book when I get done with it. I'll be doing my best to explain this. :)
|22:00:08      attiks | webchick: but that will disconnect it from the attribute as used in HTML
|22:00:27    webchick | mdrummond: YAY! I need it. :)
|22:00:28   mdrummond | webchick: No, it should really be sizes.
|22:00:32    webchick | ok, that's fine
|22:00:49    webchick | i don't want to do this more than once, which is why I'm just making sure we bikeshed early. ;)
|22:01:17   mdrummond | webchick: I'll try to find time to sketch things out. Busy week. I need to get back to the grindstone. Thanks for helping us think this through.
|22:01:45    webchick | no worries.
|22:01:46      attiks | mdrummond: let me know if you have something, or if you need help
|22:02:02    webchick | ok, so I need to summarize what we just talked about (or well someone does) in the issue
|22:02:23    webchick | which… I'm not really sure other than clarifying what the resp. image UI will be, but without a picture because that's still in mdrummond's head :D
|22:02:24      attiks | webchick: copy - paste IRC
|22:02:25         --> | alexpott (~alexpott@cpc65148-nmal17-2-0-cust166.19-2.cable.virginm.net) has joined #drupal-contribute
|22:02:28    webchick | could one of you guys do that?
|22:02:28    webchick | Hm.
|22:02:31    webchick | It's a LONG log tohgh
|22:02:44    webchick | and most of it is me askoing random questions that don't matter :D
|22:02:57    webchick | am I hearing consensus that we want to start with sizes.yml first?
|22:03:10   mdrummond | attiks: Are up for reviewing https://www.drupal.org/node/2330899 to see if it's RTBC? You have a small patch in there but you understand that one better than I do.
|22:03:11   Druplicon | https://www.drupal.org/node/2330899 => Add MIME type support for image styles #2330899: Allow image effects to change the MIME type + extension, add a "convert" image effect => 19 comments, 6 IRC mentions
|22:04:05   mdrummond | webchick: That's where I'd like to start. But I'll get some sketches made so the roadmap is clear.
|22:04:26      attiks | mdrummond: I'll mark it RTBC, so webchick can have a look as well, alex already had a look as well
|22:04:35   mdrummond | attiks: Sounds good.
|22:04:44   mdrummond | attiks: Finish line in sight!
|22:04:58    webchick | attiks++
|22:05:01    webchick | mdrummond++
|22:05:06   mdrummond | attiks++

FYI: Included the screenshot of the "UI" that - for the moment - is part of the picture module.

If you want to read up on sizes and srcset, http://ericportis.com/posts/2014/srcset-sizes/ is a good - funny - overview

We discussed this with Kevin, John, Rupl, Roy, Bohjan, ...

First prototype is in the picture module (new_ui branch)

Attached some screenshots

1/ Edit page, breakpoint label and multiplier to the left. Media query to the right

2/ Selecting an image style is the default behavior

3/ If you select 'Do not use' the image style is disabled

5/ If you select sizes, you can enter the vw using a number element

6/ Changing the width,changes the image so people say what is needed

7/ Selecting advanced mode at the bottom hides the image and show a textfield so you can go nuts

FYI: We need the following committed first

1. #2330899: Allow image effects to change the MIME type + extension, add a "convert" image effect
2. #2260061: Responsive image module does not support sizes/picture polyfill 2.2
3. #2348851: Regression: Allow HTML tags inside detail summary

Link to the repo: http://cgit.drupalcode.org/picture/tree/?h=new_ui

PS: Themes can define all image styles and image mappings, so a site builder is using this to make minor changes.

We opted for easy to understand defaults (select an image style) and hide advanced options to make it more user friendly

#18 A rough sketch would be great

Regarding the sizes.yml, I think it is too much and in the wrong place. A theme can ship with image styles and mappings so site builders can just use them. But for the the content the sizes will depend on the content, For example I add a view with an image in one column, the sizes string will depend on the number of columns I have in my view.

PS: #2349461: Move fallback image style into the responsive image style entity created as well, so needs to be a part of the interface as well

Postponing on #2260061: Responsive image module does not support sizes/picture polyfill 2.2 and #2348851: Regression: Allow HTML tags inside detail summary.

#2260061: Responsive image module does not support sizes/picture polyfill 2.2 got committed (YAY!), hence this is now unblocked :)

#25 Adding an empty breakpoint is a good idea

Issue created + patch #2424587: Make the mobile breakpoint for Bartik empty

Found this issue by searching for "picture art direction" on d.o. I do think that there are legit use cases for swapping out the whole image, for example responsive hero images. But I agree that this use case is special enough that it should be left up to contrib.

Wait... baby ducks? Kittens!

@mdrummond There is a separate issue for moving the fallback image style to the responsive image style: #2349461: Move fallback image style into the responsive image style entity

#28: ROFL at that link!

I think it is safer to wait till #2123251: Improve DX of responsive images; convert theme functions to new #type element has landed

#37 You're right, I mixed up issues. I'll try to have a look at it this week.

Spec has changed #2481635: sizes is now mandatory in the spec

#41 Totally agree. I am trying to use sizes in my theme but can't find out how to define them. Are there any examples?

@mdrummond: If you're looking for inspiration, have a look at http://drupal.org/project/picture. The 8.x-1.x branch has a form that supports sizes.

There still is a TODO in the code related to this issue:

// @todo Filter 'sizes_image_styles' to a normal array in
// https://www.drupal.org/node/2334387. For an example see
// \Drupal\Core\Block\BlockBase::validateConfigurationForm().

I'm not sure why the CMI Tests aren't throwing errors for this, but the config schema of Responsive Image defines 'sizes_image_styles' as a sequence, not a mapping. Currently (with this patch), when I create a responsive image style and export it, this is the result:

uuid: af591d7e-45a1-44d8-a849-73e090286b20
langcode: en
status: true
dependencies:
  config:
    - image.style.large
    - image.style.medium
    - image.style.thumbnail
  theme:
    - bartik
id: bartik
label: Bartik
image_style_mappings:
  -
    breakpoint_id: bartik.wide
    multiplier: 1x
    image_mapping_type: image_style
    image_mapping: medium
  -
    breakpoint_id: bartik.narrow
    multiplier: 1x
    image_mapping_type: sizes
    image_mapping:
      sizes: '(min-width:700px) 700px, 100vw'
      sizes_image_styles:
        large: large
        thumbnail: thumbnail
        medium: '0'
        '_empty image_': '0'
  -
    breakpoint_id: bartik.mobile
    multiplier: 1x
    image_mapping_type: image_style
    image_mapping: thumbnail
breakpoint_group: bartik
fallback_image_style: large

Note: for breakpoint bartik.narrow the value for 'sizes_image_styles':

sizes_image_styles:
  large: large
  thumbnail: thumbnail
  medium: '0'
  '_empty image_': '0'

This should in fact be:

sizes_image_styles:
  - large
  - thumbnail
 

The code referenced in the TODO no longer exists, but an other example can be found in \Drupal\user\AccountForm:buildEntity() (line 333, without the surrounding if statement):

$form_state->setValue('roles', array_keys(array_filter($form_state->getValue('roles'))));

Which, in our case, would result in code similar to this:

  /**
   * {@inheritdoc}
   */
  public function validate(array $form, FormStateInterface $form_state) {
    // Only validate on edit.
    if ($form_state->hasValue('keyed_styles')) {
      // Check if another breakpoint group is selected.
      if ($form_state->getValue('breakpoint_group') != $form_state->getCompleteForm()['breakpoint_group']['#default_value']) {
        // Remove the image style mappings since the breakpoint ID has changed.
        $form_state->unsetValue('keyed_styles');
      }
      else {
        $styles = $form_state->getValue('keyed_styles');
        foreach ($styles as $breakpoint_id => $multiplier_styles) {
          foreach ($multiplier_styles as $multiplier => $style) {
            $form_state->setValue(
              array(
                'keyed_styles',
                $breakpoint_id,
                $multiplier,
                'sizes_image_styles',
              ),
              array_keys(array_filter($style['sizes_image_styles']))
            );
          }
        }
      }
    }
  }

Note: untested code, written off the top of my head.

That's all I could find for now.

Small nit picks

1. +++ b/core/modules/responsive_image/src/ResponsiveImageStyleForm.php
   @@ -167,7 +195,23 @@ public function save(array $form, FormStateInterface $form_state) {
   +          if ($image_style_mapping['image_mapping_type'] == 'sizes') {
   

   ===

2. +++ b/core/modules/responsive_image/src/ResponsiveImageStyleForm.php
   @@ -167,7 +195,23 @@ public function save(array $form, FormStateInterface $form_state) {
   +          else if ($image_style_mapping['image_mapping_type'] == 'image_style') {
   

   elseif, ===

3. +++ b/core/modules/responsive_image/src/ResponsiveImageStyleForm.php
   @@ -167,7 +195,23 @@ public function save(array $form, FormStateInterface $form_state) {
   +              'image_mapping' => $image_style_mapping['image_style']
   

   trailing comma

I tested the UI for any setting I was used to in the D7 picture module. I did not find any issues creating, modifying and cloning responsive image styles. I created one that uses basically all three choices available and exported it:

uuid: 939b0642-6a1d-4042-9132-503b3dbe2d64
langcode: en
status: true
dependencies:
  config:
    - image.style.large
    - image.style.medium
    - image.style.thumbnail
  theme:
    - bartik
id: default
label: Default
image_style_mappings:
  -
    breakpoint_id: bartik.wide
    multiplier: 1x
    image_mapping_type: sizes
    image_mapping:
      sizes: 100vw
      sizes_image_styles:
        - large
        - medium
        - thumbnail
  -
    breakpoint_id: bartik.narrow
    multiplier: 1x
    image_mapping_type: image_style
    image_mapping: thumbnail
  -
    breakpoint_id: bartik.mobile
    multiplier: 1x
    image_mapping_type: image_style
    image_mapping: '_empty image_'
breakpoint_group: bartik
fallback_image_style: large

Looks good to me.

I am going to use this to create responsive image styles for my current project and I will report back any issues I face.

I found one way we could improve the UI: When using media queries in the sizes attribute, we could reuse the breakpoints from the selected breakpoint group, right? Maybe we could handle this in a follow-up?

#74 It is possible, but in most cases you'll end up using different media queries for the sizes, so presenting the list of breqkpoints defined in the theme will probably give the wrong impression. Best to create a separate isssue so this can be discussed.

I tested the patch and had another look at it, but it looks great so RTBC

Thanks all

Patch looks good. Only a silly, silly, silly nitpick to be found, which should not hold up commit.

+++ b/core/modules/responsive_image/src/ResponsiveImageStyleForm.php
@@ -167,7 +195,23 @@ public function save(array $form, FormStateInterface $form_state) {
+          else if ($image_style_mapping['image_mapping_type'] === 'image_style') {

Nit: we use "elseif".

For those of whom, like me, need a bit of a refresher on what the responsive image UI even is, here's a reminder:

Step 0: Enable the module.
Step 1: Go to /admin/config/media/responsive-image-style.

(Seems unchanged in this patch.)

Step 2: Click that add button. (admin/config/media/responsive-image-style/add)

(Seems unchanged in this patch.)

Step 3: Now you get the same form back, but with expanded options.

Before patch:

(Under those drop-downs are a list of your configured image styles, e.g. Large, Thumbnail...)

After patch (note all collapsible field sets are initially collapsed):

If you select "Image styles," States API will load a selection of configured image styles, like the old UI:

Then there's this new option for using the sizes attribute. Which also offers the image styles, this time in checkboxes:

So... I can see why this is needed and why the old UI was insufficient. But as a site builder, or heck, even as a developer (but not themer), I really just have absolutely no idea how to use this UI. :( And some of that is "pre-existing condition"... it's not like the old UI was super intuitive either. But we've now taken an unintuitive UI and added more options to it, which... hm. :(

I then went to the help page, but there's no help there either. It only mentions the issue styles option, not this new one. So at the very least, that needs updating for the docs gate. Secondarily, we might want to put some additional description text on the main form, to help understand when I'd want to use image styles vs. sizes and image styles.

Sorry, I know it's really not nice to leave things RTBC for a week and then smack 'em with a hammer. I'd love to work with one or both of you to get this into a comnittable state. I'll be on IRC intermittently tonight/tomorrow/Tuesday.

Doing what I said.

>:\

Right, I want to be clear that I absolutely want to see this go in, and I definitely am not searching for a perfect UI. However, we do need to at least pass the docs gate (which means making hook_help() accurate to the new changes), and we should also look for some relatively simple things we can do to help people who don't know the responsive image specs by heart (which is, er, most people :)) at least take a stab at the right thing to do here (to pass the usability gate as well). Linking to the spec for more info, adding field descriptions to better explain the options, and if sizes is the more important one, ordering that first... that sort of thing. Good to know that Picture module tutorials will work, but it's not really okay that people would need to run to Google in order to use an interface in Drupal core.

Ping me when you're back around and we can work together on it, if you want!

Alternative approach: what is we provide 2 edit screens, a basic to define the sizes and an advanced one as outlined above.

Basic one can be something like this:

1/ If your theme has a max width, enter the max width and enter the width of the image in px, so the browser will not try to download images that are too big.
The output will be: (min-width: 80em) 424px
2/ A table with 2 input fields per row: one for the MQ, one for the width
"Start with the widest (when using min-width MQs)
If my page is 43em wide, my image is 40 % wide
The row will get translated into (min-width: Column 1) Column 2 vw
The output will be: (min-width: 43em) 40vw
3/ On mobile my image will be 90% wide
The output will be: 90vw

Output in the browser will be: (min-width: 80em) 424px, (min-width: 43em) 40vw, 100vw

At the top of this page we can instruct the user when they might want to use the advanced edit mode:
In case you want to support the art-direction case or you want to support multiple image formats simultaneously, you can use the advanced edit mode.

What do you think?

It won't take that look, I'll have a look tomorrow to see what I can do.

#90 New issue created #2495389: Provide a simplified UI to support current responsive image standards

Teaser image

Marc and I sat together at Twin Cities DrupalCamp for at least a couple of hours walking through this UI. I got hung up on two things:

1) The terminology (e,g. Breakpoint group, fallback image) didn't make sense to me as someone who does not write front-end code, and there was no help text there to explain what I should enter there. Because these were required fields, this made me panic. :)
2) Without "smart defaults" I was left to read up on how the image srcset/sizes spec works. At this point, i'd be better off writing code than using a UI.

Marc addressed #1 with a combination of some actually helpful help text (I'd love to review this more when I can focus, aka when I'm not at a sprint), and field descriptions copy/pasted from said help text which describe what input is expected.

For #2, we came up with a plan to define a few additional image styles (Gigantic, Huge) for retina displays, and providing a couple of default responsive image styles (e.g. Narrow and Wide) that are pre-configured with the right options. This would make it much easier, because if I had questions about how the thing worked, I could simply refer to the examples, just as I can with image styles.

We also talked about other more radical proposals, however Drupal doesn't have the contextual knowledge of what the front-end is doing (e.g. margin, padding) to allow for this. And moreover since most front-end developers going forward are going to be writing IMG tags this way, abstracting it behind a UI will make them very angry. :)

Given that this is fixing a bug with the current UI, which is geared toward an older version of the responsive images spec, I think as long as we have sample content for people to learn from, I'm comfortable with this going in to resolve the major task. We can and will still do follow-up work on this UI in subsequent 8.x versions to make it more betterer.

Let's see what the test bot thinks of it so far.

One small thing I found in the documentation:

+++ b/core/modules/responsive_image/responsive_image.module
@@ -28,11 +28,23 @@ function responsive_image_help($route_name, RouteMatchInterface $route_match) {
+      $output .= '<dd>' . t('While you have the option to provide only image style per breakpoint, the sizes option allows you to provide more options to browsers as to which image file it can display, even when using multiple breakpoints for art direction. Breakpoints are defined in the configuration files of the theme.</dd>');

"[...] only image style per breakpoint [...]" should be "only one image style per breakpoint".

The tests seem to fail because the help.page route does not exist (as far as I can see).

So we need to either:

• Add the help module as dependency for the responsive images module (easiest, but I wouldn't do it, personally)
• Check for the existence of the help module before linking to a help page (harder, but more favourable IMHO)

2 small remarks by just reading through the patch:

1. +++ b/core/modules/responsive_image/src/ResponsiveImageStyleForm.php
   @@ -98,42 +102,78 @@ public function form(array $form, FormStateInterface $form_state) {
   +            'sizes' => $this->t('Select multiple image styles and use the sizes attribute.'),
   +            'image_style' => $this->t('Select a single image style'),
   +            '_none' => $this->t('Do not use this breakpoint'),
   

   Nit: Punctuation. I'd say either give all options a '.' at the end, or none of them. I'm not sure what the standard is here throughout core (if there even is one).

2. +++ b/core/modules/responsive_image/src/ResponsiveImageStyleForm.php
   @@ -98,42 +102,78 @@ public function form(array $form, FormStateInterface $form_state) {
   +          '#description' => $this->t('See the <a href="!responsive_image_help">Responsive Image help page</a> for information on the sizes attribute.', array('!responsive_image_help' => (\Drupal::moduleHandler()->moduleExists('help')) ? \Drupal::url('help.page', array('name' => 'responsive_image')) : '#')),
   

   So if the help module is disabled, there is a link here that goes nowhere?

I'll manually test the patch now but codewise it already looks good.

The patch needed a reroll, so I did that and addressed the issues I mentioned in #109.

Great job guys, patch looks good and improves the UX a lot

I will not have time to review this before next week. Glancing over the help text it is not in alignment with most of our UI standards, which clearly state that we shouldn't explain the UI - but instead only explain core concepts that are difficult to just know. I am a bit worried we keep using the same labels, and try to solve it with more descriptions.

Assigning to @webchick since her feedback drove the latest changes. Also before getting this in it would be great to have @Bojhan's review.

Ok, had a hangout today (well, actually a joint hangout/skype/phone call...(*&@#ing technology :P) with @Bojhan and @mdrummond to talk more about this.

Let's start with: All three of us are on board with the fact that we need to get this in, since the current UI is essentially a bug, because it still refers to the old picture spec.

Marc (re-)demoed the UI to us and really only three things of note came up:

- (minor, fix in this patch) There's a bit of help text on breakpoint group that is slightly lying and referencing themes vs. themes and modules.
- (major, follow-up) We need some kind of states API / ajax update for the breakpoint group form, because it is currently very confusing that you need to save the form, refresh the page, and only then see the relevant breakpoint groups in the details elements. It also can cause minor data loss if you fill out breakpoint group A, switch to breakpoint group B to check it out, and then flip back to breakpoint group A, because you need to save each time, which clears your previous settings.
- (normal, follow-up) The help text in general (both field descriptions and actual help page) does not conform to UI guidelines and needs clean-up. Specifically, it's all way too long, and it focuses on exposing the underlying technical details vs. being more use-case focused "Use this to foo your bar so your users can baz." See Field UI settings descriptions for an example.

So given that, while this UI could definitely use some pretty major refinement to be more usable to non-W3C-spec-readers at large, that can also happen in 8.1.x+ and/or contrib. Versus fixing the UI really does need to happen prior to RC.

Therefore! Fixed the bogus descriptions mentioned in the first point, and committed and pushed to 8.0.x. YEAH!!! Great work, all, on this important issue. Sorry it took quite a few twists and turns to get there. Thanks very much for your patience/persistence.

We will need follow-ups for those other two points, so adding that tag.

follow ups created:

- #2547917: Add states API / ajax update for the breakpoint group form
- #2547919: Improve help text for responsive images

Will this new UI support the use of srcset w attribute?

i.e.

<img src="https://ericportis.com/assets/2014-03-24-srcset-sizes/men-in-black/16th.png" srcset="https://ericportis.com/assets/2014-03-24-srcset-sizes/hammering/8th.png 1280w, https://ericportis.com/assets/2014-03-24-srcset-sizes/yay-peas/half.png 1880w" alt="A rad wolf"/>

http://jsbin.com/xiwuyoruce/1/edit?html,css,js,output

It is worth mentioning this should be used as an alternative to the multiplier and not in combination.

This sounds awesome. A very good reason to start using Drupal 8 in my opinion.