Skip to content
This repository has been archived by the owner on Oct 15, 2022. It is now read-only.

Coderwall: Update to handle markdown content from API (fixes #2548) #2632

Merged
merged 14 commits into from
May 12, 2016

Conversation

xaviervalarino
Copy link
Collaborator

This PR fixes the issue described in #2548

Fixes include:

  • Changing the object passed to normalize to match the new Coderwall API response
  • A custom handlebars helper to parse any markdown in description
  • Profile avatar images included in iconImage

@moollaza suggested also using chompContent to hide descriptions that are too long, but I am not sure how to implement it.

CC: @GuiltyDolphin


IA Page: https://duck.co/ia/view/coderwall

@daxtheduck
Copy link

@xaviervalarino Hey!

Thanks for taking the time to contribute! We really appreciate it.

We work closely with every contributor to make Instant Answers the best they can be, so we appreciate your patience as we look over your code. From here, the process usually goes like this:

  1. Pull Request is reviewed by the DuckDuckGo staff and community
  2. Staff and community will leave feedback with any necessary updates to the function or design.
  3. Once you've made any necessary corrections, then your Instant Answer will be merged and deployed live on DuckDuckGo!

If you have any questions along the way, feel free to ask them here. Our staff and community are also available on Slack to answer any questions you may have. If you'd like to join us there please send an email to [email protected].

More Info: https://duck.co/duckduckhack/submission_and_review

Thanks!

@daxtheduck
Copy link

Coderwall

Description: Display information about a coderwall.com user

Example Query: [coderwall jpcamara](https://beta.duckduckgo.com/?q=coderwall jpcamara)

Tab Name: Social

Source:

These are the important fields from the IA page. Please check these for errors or missing information and update the IA page


This is an automated message which will be updated as changes are made to the IA page

};

Handlebars.registerHelper("parseMD", function(content){
Copy link
Member

Choose a reason for hiding this comment

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

Did you get this from anywhere? If so it'd be useful to include a link to reference so peeps can check out more info.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is an original handlebars helper. It might be best to switch it out with something that is tried-and-true. The only search result that seemed promising to me was helper-markdown, which has remarkable as a dependency.

@moollaza moollaza changed the title Coffeecola/coderwall Coderwall: Update to handle markdown content from API Mar 29, 2016
@xaviervalarino
Copy link
Collaborator Author

I force pushed this branch. I realized doing so is strongly discouraged only after reading through the original PR for this IA. Sorry!

@GuiltyDolphin
Copy link
Member

@xaviervalarino Don't look at any of my PRs 😉

@moollaza
Copy link
Member

Deploying to beta...

@daxtheduck daxtheduck deployed to beta.duckduckgo.com March 31, 2016 16:27 Active
@moollaza
Copy link
Member

@xaviervalarino I'm getting a JS error in the console:

jpcamara_coderwall_at_duckduckgo_and_coderwall__update_to_handle_markdown_content_from_api_by_xaviervalarino_ pull_request__2632 _duckduckgo_zeroclickinfo-spice

@moollaza
Copy link
Member

@xaviervalarino false alarm, it looks like the code on Beta is still old... it's failing on item.accounts in the $.each() block which you've fixed here

@daxtheduck daxtheduck deployed to beta.duckduckgo.com March 31, 2016 16:55 Active
@daxtheduck daxtheduck deployed to beta.duckduckgo.com March 31, 2016 19:55 Active
@moollaza
Copy link
Member

@xaviervalarino alright the correct code is there now, but we have another problem. the input to the ellipsis function is causing problems. I think you're going to need to use the condense helper after all

@moollaza
Copy link
Member

https://beta.duckduckgo.com/?q=coderwall+jpcamara&ia=social

coderwall_jpcamara_at_duckduckgo

@xaviervalarino
Copy link
Collaborator Author

@moollaza I've been having trouble with both helpers!

I update the parseMD helper to sanitize the account description and avoid any potential cross-site scripting. It returns a SafeString, which fully breaks ellipsis and causes condense to not trim the string.

Even when I just return a normal string, these helpers are still counting the characters in the HTML tags, which results in incomplete and malformed tags:

condense

coderwall-condense
wraps the More at link into the unclosed tag


ellipsis

coderwall-ellipsis
writes invalid markup


line = line.replace(/\*\*(.+?)\*\*/g, '<strong>$1</strong>') // bold
.replace(/(?:_(.+?)_|\*(.+?)\*)/g, '<em>$1$2</em>') // italicize
.replace(/`(.+?)`/g, '<code>$1</code>') // monospace
.replace(/~~(.+?)~~/g, '<del>$1</del>') // strikethrough
Copy link
Member

Choose a reason for hiding this comment

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

If you need any help with HTML escaping check out MarkdownReference - I found a pretty good helper for it a while back.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for this! I can definitely see myself referring to those entities in the future. What do you mean by escaping the HTML, though? Wouldn't escaping it turn the description into a garbled mess of markup + text?

Copy link
Member

Choose a reason for hiding this comment

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

For code blocks you can use it to ensure HTML doesn't decide to take it upon itself to start changing things. If you look in the Handlebars for MarkdownReference you'll see what I mean.

You might not need it, but it is there if you do.

@xaviervalarino
Copy link
Collaborator Author

I've been having trouble with both helpers!

@moollaza I have a couple of ideas for how to work around this:

Short Term

  1. Exclude built in helpers: don't trim the content and leave it as is
    • UPSIDE: Easy to implement, could get to production faster
    • DOWNSIDE: Longer descriptions could eat up most of the viewport, taking away from search results below
  2. Create a custom helper for the interim (see long term) to trim this specific type of string. I have been working on block helper that will do this, but haven't pushed any commits yet.
    • UPSIDE: Effective trimming that only counts visible characters and excludes markup
    • DOWNSIDE: More moving parts/ more likely to break

Long Term

Create issues and/or PRs to ellipsis and condense to fix the following issues:

  • Better tag handling
  • Support SafeStrings (I already created DDG template-helpers Issue #9)
  • Propose an enhancement to optionally exclude markup, e.g.,
{{condense (parseMD description) maxleng='300' ignoretags='true' truncation='...'}}
...
{{ellipsis (parseMD description) 300 ignoreTags}}

Then come back and clean up the Codewall IA

(Sorry if this is a little didactic, I just had the impression we wanted to get this out the door sooner rather than later since #2548 is marked as a high priority issue.)

@moollaza
Copy link
Member

@xaviervalarino thanks again for improving the Coderwall Spice! You’ve been doing a really good job here and we'd like you to help our community tackle the next big challenge - our mission to become the best search engine for programmers: https://duck.co/blog/post/297/help-for-programmers.

When you're ready to move on, please:

  • Create a new Instant Answer to increase our Programming IA coverage

OR

  • Test live programming Instant Answers (or those in development) for bugs and suggest improvements

Detailed instructions explaining how you can help are available at the top of this page.

Thanks again!

@xaviervalarino
Copy link
Collaborator Author

xaviervalarino commented Apr 26, 2016

Finally got around to trying to close this PR, but have run into an issue that I don't know how to fix.

coderwall-spice-failed
The console says that the Coderwall Spice has failed:

[NOTICE] Spice.failed() called by Spice with ID "coderwall"

I thought the issue might be caused by some of my new commits so I checked out an earlier one, but still get the same issue.

It also looks like the Coderwall API page is down. Maybe they changed/removed the API?

It looks like it's still sending back the same response:
https://coderwall.com/jpcamara.json?full=true&callback=ddg_spice_coderwall
Maybe it has something to do with the perl file?


EDIT: Coderwall changed the API object (again).

latest commit fixes this issue.

Coderwall has changed some of the keys on their API.
  ( i.e.,
    api_result.data       --> api_result.user
    api_result.avatar.url --> api_result.thumbnail
  )
Small changes, but frustrating to have to fix them in
less than a 2 month time span.
- Cleans up some soft tabs from 2 to 4
- Handlebars helper is registered with Spice
- Handlesbars helper is namespaced
Some of the older user avatars on coderwall have a 20px border
around them that looks god-awful with the iconImage template.

This CSS file enlarges the image ~120% in order hide that border.
@xaviervalarino
Copy link
Collaborator Author

xaviervalarino commented Apr 26, 2016

@zachthompson I remember reading in your PR that there was some issues with the white border on the profile images for older account.

The custom CSS in the latest commit should fix this.

coderwall-custom-css

@xaviervalarino xaviervalarino changed the title Coderwall: Update to handle markdown content from API Coderwall: Update to handle markdown content from API (fixes #2548) Apr 28, 2016
This commit adds minor fixes to the Markdown Handlebars
helper. Specifically, the replace methods used to parse
italics and bold.

Coderwall's markdown parser seems to be based on Github's.
Although italics and bold don't result in any visual change
on their site, the parser is still marking up the <em> and
<strong> tags.

So, as of writing this, Coderwall will markup this syntax:

  *word*       -->  italic
  _word_       -->  italic
  __word__     -->  bold
  **word**     -->  bold
  `word`       -->  monospace/code
  ~~word~~     -->  strikethrough
  [word](url)  -->  link

They are also parsing ordered/unordered lists and headers.
My inclination is to not parse these, as it seems (anecdotally)
that this is not commonly used by users when they author their
descriptions, and, mainly, because rendering the lists and
headers will very quickly bloat div.cz-main and eat up a lot
of vertically space.

Coderwall does _NOT_ parsing images or tables.
{{#coderwall_substr_html}} is a custom block helper that trims
HTML text to a specific length. It does this by parsing the
context/string into HTML. The helper then iterates through each
text node, and then, if the textContent is longer than the
specified character limit, the text is trimmed to the closest
word. Once the limit is hit, all remaining nodes are exluded.

Ideally, this custom helper will be a temporary solution until the
built-in {{#condense}} block helper supports markup.
@xaviervalarino
Copy link
Collaborator Author

xaviervalarino commented May 3, 2016

@moollaza i've created a custom block helper that trims any description over 250 characters to the closest word. I also created a Codepen that uses these helpers so that you can play around with different character limits and custom markdown.

That sort of closes out everything I wanted to do for this IA.

@daxtheduck daxtheduck deployed to beta.duckduckgo.com May 11, 2016 16:21 Active
@moollaza
Copy link
Member

LGTM 👍

@moollaza moollaza merged commit 467ace0 into duckduckgo:master May 12, 2016
@xaviervalarino xaviervalarino deleted the coffeecola/coderwall branch July 13, 2016 07:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants