#332 ✓ staged
David Humphrey

Create a LinkedIn plugin

Reported by David Humphrey | February 12th, 2011 @ 05:21 PM | in 0.6

Investigate what we can query and display from LinkedIn in a plugin.

Comments and changes to this ticket

  • danman

    danman April 11th, 2011 @ 10:31 PM

    • State changed from “bugs” to “peer-review-requested”
    • Assigned user set to “danman”

    Here's a plugin that uses LinkedIn plugins:
    https://github.com/DanVentura/popcorn-js/commit/7443a2960ca27cbd09b...

    For a demo, go here:
    http://matrix.senecac.on.ca/~dsventura/popcorn-js/plugins/linkedin/...

    The reason why the demo is on matrix, and the plugin doesn't work locally, is because LinkedIn plugins require an api_key for your domain. The api_key included in my branch is valid for all of matrix. If an api_key isn't specified, I made it so a handy error message with a link will allow you to get one.

    Here's a description of the plugins:
    http://developer.linkedin.com/community/plugins

  • Rick

    Rick April 20th, 2011 @ 10:32 AM

    • State changed from “peer-review-requested” to “review-needs-work”
    • Milestone set to 0.6
    • Milestone order changed from “30” to “0”

    Please provide unit tests - if you need help, take a look at the other plugins and don't hesitate to ask for help in the popcorn irc channel

  • Rick

    Rick April 20th, 2011 @ 10:35 AM

    I'm getting this error when running the demo page:

    "Uncaught Error: JavaScript API Domain is restricted to matrix.senecac.on.ca"

    ...I'm assuming this is related to the required API key.

  • annasob

    annasob May 5th, 2011 @ 01:18 PM

    Another awesome plugin Dan!
    The only major problem is:

    var api_key = 'ND1uSIEUBH8MkI8E7g41kgGlWUexzvFL9uB2ihJbIrFuqunFq8aVmUrxyfAqxCBX';
    

    We need to allow the user of the plugin to put in their own api_key which means it needs to be an option. Look at the flickr plugin to get an idea. Is there a way that you can make a new api_key for all domains instead of matrix specific? Also we need a disclaimer saying this api key is only for testing uses (find that in flickr plugin as well). I see you are using a global but polluting the global scope is not a good practice.

    • unit tests needed
    • plugin needs start and end
    • more option documentation needed at the top similar to comments in #331
  • Mohammed Buttu

    Mohammed Buttu May 11th, 2011 @ 09:04 AM

    • Assigned user changed from “danman” to “Mohammed Buttu”
  • Mohammed Buttu

    Mohammed Buttu May 15th, 2011 @ 11:20 PM

    • State changed from “review-needs-work” to “peer-review-requested”
    • Assigned user changed from “Mohammed Buttu” to “Rick”

    [branch t332] (https://github.com/mbuttu/popcorn-js/tree/t332/plugins/linkedin)

    I have completed most of this, but if I can get help with the list below, that would be great:

    • We may need to add a unit test to check if the web page is being run on localhost (API key used must be run on localhost)
    • I'm not sure if the documentation is complete
    • For the unit tests, they all pass, but the plugin is not showing up on the web page
    • The plugins do not always show up on first load, but they do on subsequent loads

    Thank you,
    Mohammed

  • Rick

    Rick May 16th, 2011 @ 01:50 AM

    • State changed from “peer-review-requested” to “review-needs-work”

    Please bring into conformance with: https://webmademovies.lighthouseapp.com/projects/63272/styleguide

    If you have any questions, feel free to ask here or in the #popcorn channel, Thanks!

  • Scott Downe

    Scott Downe May 16th, 2011 @ 09:35 AM

    Regarding the localhost test. An api key for linkin apparently needs to be registered with one url, we made one for testing purposes that is registered with localhost.

    We need a test that will politely fail if that is not the case. Here is the basic framework to accomplish that:

    
    if ( /localhost/.test( location.protocol ) ) {
    
      // run localhost required tests here
    
    } else {
    
      // failure
      ok(false, "linkin apikey will only work under localhost");
    
    }
    

    The main idea here is to give back information relative to the problem, instead of just stalling the tests, but also, to continue testing where applicable.

  • Scott Downe

    Scott Downe May 16th, 2011 @ 09:37 AM

    Regarding documentation, it is missing url and counter in the /** paragraph.

    Might be more, that was at a glance.

    Regarding style, I understand it's not all your code, you picked this ticket up half way through, but it is new code, that wasn't there before, so it does need to conform to the style guide.

  • Mohammed Buttu

    Mohammed Buttu May 16th, 2011 @ 02:18 PM

    • State changed from “review-needs-work” to “peer-review-requested”
  • Rick

    Rick May 16th, 2011 @ 04:47 PM

    • State changed from “peer-review-requested” to “review-needs-work”
    • Assigned user changed from “Rick” to “Mohammed Buttu”

    There is a lot of quote mixing, can you please update the single quotes to use double quotes. Thanks!

    Also, I'm seeing passing tests and those tests look good, but I don't see any "linkedin" widgets in either the test suite or the demo page :\

  • Mohammed Buttu

    Mohammed Buttu May 17th, 2011 @ 12:01 PM

    • State changed from “review-needs-work” to “peer-review-requested”
    • Assigned user changed from “Mohammed Buttu” to “Rick”

    Code style has been corrected and quotes have been changed to double quotes (except where single quotes are needed).

    [branch t332] (https://github.com/mbuttu/popcorn-js/tree/t332/plugins/linkedin)

    Rick, were you running the tests/demo on localhost, port 80? If not, then not all of the widgets would have been displayed. If you were running on localhost, port 80, what browser were you testing this on?

    Thank you,
    Mohammed

  • Rick

    Rick May 17th, 2011 @ 12:21 PM

    • State changed from “peer-review-requested” to “super-review-requested”
    • Assigned user changed from “Rick” to “Scott Downe”

    I always run tests over http on a localhost. I must've confused branches or something. This works great. Nice job on the code clean up

    Tested linkedin test suite

    • FF 3.6, 4.x, 6.x

    • Chrome 11, 12, 13

  • Mohammed Buttu

    Mohammed Buttu May 17th, 2011 @ 01:27 PM

    • State changed from “super-review-requested” to “peer-review-requested”
    • Assigned user changed from “Scott Downe” to “Rick”
  • Rick

    Rick May 17th, 2011 @ 01:38 PM

    • State changed from “peer-review-requested” to “review-needs-work”
    • Assigned user changed from “Rick” to “Mohammed Buttu”

    I feel bad bringing this up after I already PR+'ed your work, but I somehow completely missed it...

    
    (function() {
    
      var linkedinAPIScript = document.createElement( "script" );
      linkedinAPIScript.setAttribute( "src", "http://platform.linkedin.com/in.js" );
      linkedinAPIScript.setAttribute( "type", "text/javascript" );
      linkedinAPIScript.setAttribute( "async", "true" );
      root.appendChild( linkedinAPIScript );
    }());
    

    Should be:

    
    Popcorn.getScript("http://platform.linkedin.com/in.js", function() {
      // optional callback
    });
    

    Sorry I missed this before :\

    I also want to note that you're persistance and patience working through this ticket is greatly appreciated, you're a champ! :D

  • Mohammed Buttu

    Mohammed Buttu May 17th, 2011 @ 02:15 PM

    • State changed from “review-needs-work” to “peer-review-requested”
    • Assigned user changed from “Mohammed Buttu” to “Rick”

    Changed the code that creates the LinkedIn API script to make use of Popcorn.getScript.

    [Commit] (https://github.com/mbuttu/popcorn-js/commit/2659c41aa6185ea14071016...) [branch t332] (https://github.com/mbuttu/popcorn-js/tree/t332/plugins/linkedin).

    Thank you for the help, Rick. The Popcorn.getScript method is awesome.

  • Rick

    Rick May 17th, 2011 @ 06:59 PM

    • State changed from “peer-review-requested” to “super-review-requested”

    Great work!

    Tested linkedin test suite, passing

    • FF 3.6, 4.x, 6.x

    • Chrome 11, 12, 13

    (again)

  • Rick

    Rick May 18th, 2011 @ 09:24 AM

    • Assigned user changed from “Rick” to “annasob”
  • annasob

    annasob May 18th, 2011 @ 10:40 AM

    • State changed from “super-review-requested” to “review-needs-work”
    • Assigned user changed from “annasob” to “Mohammed Buttu”
    • Milestone order changed from “5” to “0”

    Mo, some more things to fix
    - linkedin.js

               :line 5 needs a tab
               : line 18 doubble "is"
               : missing _teardown function anything added to the dom should be removed:
    
    _teardown: function( options ) {
            document.getElementById( options.target ) && document.getElementById( options.target ).removeChild( options._container );
          }
    
    • linkedin.html

                : please make some of the stuff appear for a longer period with a shorter interval in between showings 
                : in the page comments tell the user at what time things should appear/disappear
      
    • linkedin.unit.html : the title of the page is Popcorn API change to Popcorn LinkedIn Plugin Test

    • plugins/index.html is missing a link to this plugin

  • Scott Downe

    Scott Downe May 18th, 2011 @ 10:45 AM

    One not about teardown.

    There is a better way to do it than how I was doing it:

    var tar = document.getElementById( options.target );
    tar && tar.removeChild( options._container );
    
  • Mohammed Buttu

    Mohammed Buttu May 18th, 2011 @ 12:47 PM

    • State changed from “review-needs-work” to “super-review-requested”
    • Assigned user changed from “Mohammed Buttu” to “annasob”
  • annasob

    annasob May 18th, 2011 @ 01:15 PM

    • State changed from “super-review-requested” to “review-looks-good”
    • Assigned user changed from “annasob” to “Mohammed Buttu”

    this looks good now! SR+

  • annasob

    annasob May 18th, 2011 @ 01:16 PM

    • State changed from “review-looks-good” to “staged”

    Staged in annasob/popcorn-jscommit

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Popcorn.js is an HTML5 video framework that lets you bring elements of the web into your videos.

Popcorn.js is a project of Web Made Movies, Mozilla's Open Video Lab.

Shared Ticket Bins

Tags

Pages