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

Removes HTML tags from feed titles #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dsummersl
Copy link

@bcipolli
Copy link

bcipolli commented Mar 6, 2016

Thank you! I'm not sure this will give properly linked urls in each scenario: when viewing the citygram map, in the email, and when viewing the email in the browser.

Can you confirm that this will lead to properly linked items in each scenario? I'm a bit suspicious that maybe not, as there are no a tags, so nothing related to linking to strip out.

@dsummersl
Copy link
Author

hmm, good question. I'll check. The main citygram app 'hyperlinks' plain text, so I think...as long as this text is plain text and has a properly formatted URL it should be okay.

@dsummersl
Copy link
Author

Okay! Spent a little time over the weekend on this. You're totally right, this is much more complicated...

  • Was able to setup citygram to pull from my locally hosted version of this PR - yay!
  • The citygram app has two hyperlink() methods - one in javascript (Autolinker.js) and one in ruby (erb I...think). The JS version works great if you have text with a link in it: it will find a link in the text, and wrap an <a> tag around it. The ruby version does not: it just wraps a link around the full text, which sometimes renders badly.
  • I looked through the other publishers and it look like san diego is the only publisher with a link in the published text.
  • I saw a couple another thread you opened, and some suggested improvements options: Pass raw event values to front-end css codeforamerica/citygram#244 -- I think this issue will require some change to the geojson format that is published to truly fix it. I'll make comments over on that thread...seems like support needs to be in citygram before its worth finalizing this PR.

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.

2 participants