Closed (fixed)
Project:
Drupal core
Version:
8.1.x-dev
Component:
responsive_image.module
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
26 Jan 2016 at 19:11 UTC
Updated:
11 Jul 2016 at 15:54 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
claudiu.cristeaHere's a patch.
Comment #3
claudiu.cristea@attiks, maybe you'll find some time to look into this.
Comment #4
claudiu.cristeaOuch, the test.
Comment #5
wim leersLooks great!
Nit: s/configurations/configuration/
Nit: s/wa snot/was not/
Comment #6
alexpott8.0.x has now had its final release.
Comment #7
alexpottShouldn't this be using the replacement image style if the image style gets deleted.
Comment #8
claudiu.cristea@alexpott, hm. I thought the same. But now I'm not sure.
Comment #9
catchComment #10
Orizontal commentedtesting the issu
Comment #11
Orizontal commentedComment #12
rainbowarrayLet's test to see what is happening right now:
Comment #13
wim leersNo, because that would mean it's no longer a responsive image style. The fallback image style is specifically intended for the case of the browser not supporting responsive images. It is not okay to just make all images using a certain responsive image style use the same image style always (i.e. the fallback).
Comment #15
wim leersPatch no longer applies.
Comment #16
claudiu.cristeaI fixed also the typos/nits from #5.
Comment #17
wim leersThanks!
Comment #19
wim leersRandom migrate fails strike again!
Comment #20
alexpottwe can do less here by only saving if the dependencies are different - see views_post_update_serializer_dependencies().... something like:
Comment #21
joginderpcRe-roll patch @claudiu.cristea with resolve given solution in last comment #20.
Comment #22
claudiu.cristea@alexpott, here is the change requested in #20 even it seems to me a little... over-engeneering for a one time task :)
Comment #23
dawehnerThis is indeed a nice pattern!
Let's ensure to remove them right before the comment.
For future reference, you could leverage
$this->assertArrayHasKey().Comment #25
claudiu.cristeaThank you @dawehner. I fixed based on suggestions from #23.
Comment #26
claudiu.cristeaBack to RTBC as per #23.
Comment #27
dawehnerBack to RTBC. Thank you @claudiu.cristea
Comment #30
catchCommitted/pushed to 8.2.x and cherry-picked to 8.1.x, thanks!