humaan / Modaal

An accessible dialog window library for all humans.
http://humaan.com/modaal/
MIT License
2.72k stars 183 forks source link

type: image content_source is useless #116

Open codefalse opened 5 years ago

codefalse commented 5 years ago

This's my options

{
        "type": "image",
        "content_source": "image's url"
}

but the code in line 685, to check data-modaal-content-source, href, src, or error.

if ( self.$elem.attr('data-modaal-content-source') ) {
    this_img_src = self.$elem.attr('data-modaal-content-source');
} else if ( self.$elem.attr('href') ) {
    this_img_src = self.$elem.attr('href');
} else if ( self.$elem.attr('src') ) {
    this_img_src = self.$elem.attr('src');
} else {
    this_img_src = 'trigger requires href or data-modaal-content-source attribute';
    img_src_error = true;
}

so, My configuration(options.content_source) is useless. We should give priority to self.scope.source,

if (self.scope.source){
    this_img_src = self.scope.source;
} else if ( self.$elem.attr('data-modaal-content-source') ) {
    this_img_src = self.$elem.attr('data-modaal-content-source');
......
danhumaan commented 5 years ago

hey @codefalse thanks for raising this. While we define and attach the root source on line 102, that doesn't apply to the image type, which definitely looks like we aren't checking if the option exists outside of it being an data attribute. We'll work this fix into the next milestone release as it should be pretty straight forward (also needs attention from line 615 as well).

As i'm not sure when that roll out might be, our recommendation for getting content_source option to work in an Image or Gallery type would be to use the data-modaal-content-source.