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

Fixes custom image header templates #3378

Merged
merged 3 commits into from
Apr 28, 2015

Conversation

alonsogarciapablo
Copy link
Contributor

Fixes #2235 and hides the <img> tag when an infowindow doesn't have a valid cover URL (different issue).

image_infowindows

cc/ @cshirky

var url = this._getCoverURL();

if (!this._isValidURL(url)) {
$imageNotFound.fadeIn(250);
$img.hide();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were not hiding the <img> tag before, and it was displaying a broken image (bonus)

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@alonsogarciapablo alonsogarciapablo changed the title 2235 fix custom image header templates Fixes custom image header templates Apr 27, 2015
@alonsogarciapablo
Copy link
Contributor Author

Hey @xavijam, when you have a second, I'd love to have your thoughts on this PR. Thanks!


var $cover = this.$el.find(".cover");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...this.$('.cover')...

@xavijam
Copy link
Contributor

xavijam commented Apr 28, 2015

Shouldn't we change the same function in cartodb.js infowindow file?
https://github.com/CartoDB/cartodb.js/blob/develop/src/geo/ui/infowindow.js#L558

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@alonsogarciapablo
Copy link
Contributor Author

@xavijam I fixed the selectors.

As per your other comment, I'd say that should also be changed in cartodb.js, but since nobody has complaint about it, I think we can wait.

@xavijam
Copy link
Contributor

xavijam commented Apr 28, 2015

Open a ticket please :)

@alonsogarciapablo
Copy link
Contributor Author

@xavijam: There you have it. Thanks!

alonsogarciapablo added a commit that referenced this pull request Apr 28, 2015
…mplates

Fixes custom image header templates
@alonsogarciapablo alonsogarciapablo merged commit b092d92 into master Apr 28, 2015
@alonsogarciapablo alonsogarciapablo deleted the 2235-fix-custom-image-header-templates branch April 28, 2015 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editing HTML in the 'Image Header' Infowindow breaks the image
3 participants