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

type: image content_source is useless #116

Open
codefalse opened this issue Nov 17, 2018 · 1 comment
Open

type: image content_source is useless #116

codefalse opened this issue Nov 17, 2018 · 1 comment
Assignees
Labels
Milestone

Comments

@codefalse
Copy link

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
Copy link
Collaborator

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.

@danhumaan danhumaan self-assigned this Nov 19, 2018
@danhumaan danhumaan added the bug label Nov 19, 2018
@danhumaan danhumaan added this to the v0.4.5 milestone Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants