#350 ✓ staged
annasob

tumblr popcorn plugin

Reported by annasob | February 18th, 2011 @ 11:21 AM | in 1.1 (closed)

We need to investigate the posibility of using popcorn in our blogs. Brett uses tumblr and it would be great if we can actually use popcorn instead of just talking about it. Most of the video found here: http://brettgaylor.tumblr.com/post/3347474662/web-made-movies-2011 could have been made using popcorn, especially the shots of wired and gizmodo.

Comments and changes to this ticket

  • annasob

    annasob February 23rd, 2011 @ 04:51 PM

    • Tag changed from tumblr to plugin, tumblr
  • David Seifried

    David Seifried September 14th, 2011 @ 07:19 PM

    • Tag changed from plugin, tumblr to plugin, starters, tumblr
    • Milestone set to 1.0 Release
    • Milestone order changed from “37” to “0”
  • mjschranz

    mjschranz September 18th, 2011 @ 09:55 PM

    • State changed from “new” to “assigned”
    • Assigned user set to “mjschranz”
  • mjschranz

    mjschranz September 20th, 2011 @ 12:16 PM

    Before I actually figure out how to read the javascript in other plugins and then go on about writing some myself, I wanted to sort of talk about the plugin just to get an idea of what is expected out of it.

    http://www.tumblr.com/docs/en/api/v2

    Obviously the idea is that the user would be able to inject information or parts of their blog hosted on tumblr during the video they have playing with popcorn. Some of the stuff the API has ranges from general information about the blog itself, the followers of the blog to a whole lot of info about the individual posts themselves.

    At this point I'm looking to allow people to bring up info about their blogs, followers and their avatar just as a basic start since I have our own 0.1 milestone for OSD600 due next Thursday.

    Anyway, am I sort of on the right track for what is expected out of the plugin? Thanks!

  • David Seifried

    David Seifried September 20th, 2011 @ 12:26 PM

    Yea you are definitely on the right track for this. The only stuff that api that I think you don't need to implement would be the posting stuff ( new post, edit post, delete post, etc ).

    The first thing I would be to just get something working and update the ticket with that. I'd probably start with just generally show there blog in a div, as thats the most basic functionality. By the look of it you are also going to need to get an API key as well.

    But yea this should be a sick plugin, can't wait to take a first look.

  • David Seifried

    David Seifried October 7th, 2011 @ 03:24 PM

    • Milestone changed from 1.0 Release to 1.1
    • Milestone order changed from “38” to “0”
  • mjschranz

    mjschranz November 14th, 2011 @ 05:19 PM

    Wanted input for dealing with retrieving photo posts. As it stands each post will only ever have one photo in it but the returned JSON contains all kinds of various sizes for the photo in the post itself.

    Basically I need a way for the user to specify what size they want to use to be displayed. As it stands my ideas were forcing them to provide width and height and searching through the alt_sizes objects with matching values. It would work but could be annoying for the user, especially if they wanted to actually obtain the original size of the picture as it is not in that same collection of objects.

    Thoughts? Suggestions?

    An example of what the returned alt_sizes objects looks like.
    http://pastie.org/2864144

  • David Seifried

    David Seifried November 15th, 2011 @ 09:25 AM

    Are all of the sizes for the alt images the same everytime? If so maybe default to a standard one, but have an optional parameter to allow the user to pass in a width and height if they want to.

  • mjschranz

    mjschranz November 15th, 2011 @ 09:49 AM

    They don't specify in the API but here are my results after making a posts with 2 different size pictures in it.

    There is a set target width for the different sizes of photos and if the width of that size is smaller than the original width of the photo then the alt_sizes set of objects will have a photo of that width but the height won't always be the same.

    So yeah for now I'll simply choose a default one (maybe 500 pixel width to not make it too big) and then allow them to specify a width and if that isn't in the alt_sizes objects it will default it to 500.

  • mjschranz

    mjschranz November 16th, 2011 @ 04:36 PM

    • Assigned user changed from “mjschranz” to “cadecairos”
    • State changed from “assigned” to “peer-review-requested”

    Finally! My first working/acceptable version is up.

    My branch can be found here and all four files will need to be reviewed obviously.

    Tumblr API

    A few things to note:

    1. I know that it's probably really ugly code and I'm open to suggestions to make it more efficient/pretty on the eyes but I feel I did the best with what I knew. I made sure as much as possible to not have duplicate code for all the different response types that shared similar attributes.
    2. I want to make the chat blogpost types better by maybe making each dialogue item appear delayed on a timer as if it was a chat happening in real time.
    3. I have no idea how the artist info will look with audio blogposts because whenever I tried to setup a test to retrieve them I could never get a response that included the info. They are response attributes according to their API but *shrug*.
    4. Followers request. At this point I went with not including it as a feature at all because to me it doesn't seem like something people would really want to use. All that is returned are "User" objects that contain the persons name, their blog url and the amount of time in seconds they last updated their blog since the epoch (huh?)

    Anyway, that's all. I want to know what can be improved!

  • cadecairos

    cadecairos November 18th, 2011 @ 11:55 AM

    • State changed from “peer-review-requested” to “review-needs-work”
    • Assigned user changed from “cadecairos” to “mjschranz”
    • Milestone order changed from “1” to “0”

    Cool plugin!

    Just a few notes:

    • Use Popcorn.error instead of throw new Error (), that way you can eliminate the if's i.e.

      !validType( options.requestType ) && popcorn.error( "Invalid tumblr plugin type." );

    • here I'd suggest swapping the conditions

    • DO NOT USE SWITCH-CASE!! haha. no but seriously, here's a better model you should replace it with:
    var doSomething = {
      one: function( a ) { console.log( a ); },
      two: function( a ) { console.log( a ); },
      three: function( a ) { console.log( a ); },
      four: function( a ) { console.log( a ); }
    }
    
    var selection = "three";
        
    doSomething[ selection ]( "switch-case blows" );
    
    • use curly braces here
    • type is leaking on the lines above too
    • this line and the line below need whitespace
    • the variable n here can be better named
    • if this is an array then you shouldn't use for...in same goes for the one below it.
    • try to cache the item being handled in each iteration rather than access it by index each time.
    • m is leaking here
    • rogue console.log here
    • don't use for..in here and wrap in curly braces
    • clean this up a bit, and turn:

    var width = !options.photoWidth ? 400 : options.photoWidth

    into...

    var width = options.photoWidth || 400

    look for all missing curly braces..
    change all for..in's for arrays to for( var blah = 0, length = something.length; blah < length; blah++ ) ... etc.

    in start and end check for the existence of options._container

    add teardown tests plzz

    That is all for now.

  • mjschranz

    mjschranz November 18th, 2011 @ 04:57 PM

    • State changed from “review-needs-work” to “peer-review-requested”
    • Assigned user changed from “mjschranz” to “cadecairos”

    Fairly certain I have caught most of them. Let me know.

    Repo
    Commit

  • cadecairos

    cadecairos November 22nd, 2011 @ 11:21 AM

    • State changed from “peer-review-requested” to “review-needs-work”
    • Assigned user changed from “cadecairos” to “mjschranz”

    Looking way better!

    few things:

    • line 3: remove space between function and (
    • lines 31,32 extra whitespace
    • line 38 end of line whitespace
    • line 39 remove this commented line
    • line 40 end of line whitespace
    • line 41 remove commented line?
    • line 43, 44 end of line whitespace
    • line 44 move definitions onto separate lines
    • line 46 extra whitespace
    • line 48 add a space between for and ( and between ending ) and opening { Do this for all for loops please!
    • line 48 cache the length of the array - i.e. for ( var i = 0, len = a.length; i < len; i++ ) { ... } Do this for all for loops please!
    • I'm too lazy to keep looking so go through and remove all EOL whitespace.
    • line 52 there's something weird going on with indentation after this point.
    • **ensure all if statements have proper whitespace i.e. if ( foo === bar ) { ... }
    • line 98 fix whitespace here, clean it up by separating onto new lines watch the whitespace :P
    • If the audio stuff doesn't work, we shouldn't support it.
    • line 107 split onto new lines
    • line 165 whitespace fixes plz
    • why does the plugin fail if there's http:// in the url? why not strip it out if detected via regex?
    • line 241 extra blank line
    • line 258 declare at top of block
    • line 262/263 make this } else { **do this for all else keywords
    • line 266 this type variable is leaking / was not declared. do this at the top of the block
    • check all uses of array notation for proper whitespace i.e. something[ foo ]
    • line 305 use Popcorn.error()

    almost there!

  • mjschranz

    mjschranz November 22nd, 2011 @ 02:52 PM

    Fixed all of that as far as I can tell and allowed for the entry of base_hostnames with http:// before them.

    As far as I know the audio stuff DOES work, at least their API claims it does. http://www.tumblr.com/docs/en/api/v2#audio-posts

    It says that its returns:

    album_art String Location of the audio file's ID3 album art image
    artist String The audio file's ID3 artist value
    album String The audio file's ID3 album value
    track_name String The audio file's ID3 title value
    track_number Number The audio file's ID3 track value
    year Number The audio file's ID3 year value

    But as I said in the comment, that information doesn't seem to be included every time and I was never able to make a test that provided it.

    For now everything else is done other than that silly loop error I showed Chris. I'll try to figure it out for a little while longer.

  • mjschranz

    mjschranz November 23rd, 2011 @ 12:31 PM

    • State changed from “review-needs-work” to “peer-review-requested”
    • Assigned user changed from “mjschranz” to “cadecairos”

    At cadecairos request I put everything I have for now in a separate branch and I'll see if he can figure it out.

    https://github.com/mjschranz/popcorn-js/blob/t350b/plugins/tumblr/p...

    Just a quick summary, for whatever reason when I try to use a cached length of the array for a condition it fails and will always run through once more than it should but when checking the length of the array each time in the condition it doesn't. I put what works in a comment above it on line 44.

  • mjschranz

    mjschranz November 23rd, 2011 @ 01:45 PM

    Found it thanks to Chris. Go figure it was some incredibly stupid error on my part.

    Hopefully this is my final submission for review!

    https://github.com/mjschranz/popcorn-js/tree/t350/plugins/tumblr

  • cadecairos

    cadecairos November 29th, 2011 @ 10:21 AM

    • State changed from “peer-review-requested” to “review-needs-work”
    • Assigned user changed from “cadecairos” to “mjschranz”

    sorry for the delay, was working on Popcorn Maker for a bit there.

    This is looking better and better. But there are a few more things that need attention:

    split up var declarations onto multiple lines for readability (but don't use more than one var keyword)

    lint bails partway through your plug-in, so run the command "make lint-plugins" and fix all the problems it reports

    in popcorn.tumblr.html:

    use double quotes, except around the source type tags.

  • mjschranz

    mjschranz December 1st, 2011 @ 12:50 PM

    • State changed from “review-needs-work” to “peer-review-requested”
    • Assigned user changed from “mjschranz” to “cadecairos”

    Fixed all lint test issues, var placements and double quote usage in the .html demo. Also realized I wasn't checking for https:// when looking at the provided blog_hostname so that is added in as well.

    Fixed as far as I can tell.

    https://github.com/mjschranz/popcorn-js/commit/ba6c49f641a623f036f1...

  • cadecairos

    cadecairos December 7th, 2011 @ 02:07 PM

    • Assigned user changed from “cadecairos” to “David Seifried”
    • State changed from “peer-review-requested” to “super-review-requested”

    I can confidently PR+ this now. Passing to dseif!

    Good work mjschranz!

  • David Seifried

    David Seifried December 9th, 2011 @ 03:15 PM

    • State changed from “super-review-requested” to “review-needs-work”
    • Assigned user changed from “David Seifried” to “mjschranz”

    So close to being done!, but a few things:

    popcorn.tumblr.js:

    • end of line whitespace on lines 40, 48, 68, 93, 94, 105, 220, 221, 227, 251, 256, 272, 278
    • Lines 45 - 85 indentation is a bit off
    • photo.alt_sizes is used > 1 time, please store a reference to it
    • options.post is used >1 time, please store a reference
    • options.post.dialogue and others are used > 1 time, check file over for where you access objects properties > 1 time
    • In situations where you are using innerHTML to create elements via a string I think it is better practice to actually create a new element, add data to its specific properties, and then append it to the parent.

    Other than that this is awesome, unit tests look solid, passes lint, works on FF 3.6, FF Current, Chrome, Safari, Opera.

  • mjschranz

    mjschranz December 14th, 2011 @ 01:50 PM

    • State changed from “review-needs-work” to “super-review-requested”
    • Assigned user changed from “mjschranz” to “David Seifried”
  • David Seifried

    David Seifried December 14th, 2011 @ 05:12 PM

    • Assigned user changed from “David Seifried” to “cadecairos”
    • State changed from “super-review-requested” to “review-looks-good”

    Awesome! Passes lint, passes in FF Chrome Opera and Safari. Unit tests and code look good.
    SR+!

  • cadecairos

    cadecairos December 14th, 2011 @ 05:15 PM

    • State changed from “review-looks-good” to “staged”
    • Assigned user changed from “cadecairos” to “mjschranz”
  • Rick
  • Rick
  • Rick
  • Rick
  • Rick
  • Rick
  • Rick

    Rick December 15th, 2011 @ 04:19 PM

    (from [512991eb8bc3e1c5f459c896a69c4b836917b0c0]) [#350]Added tumblr.unit.js file and tumblr.unit.html file. Fixed careless mistakes in tumblr.js https://github.com/rwldrn/popcorn-js/commit/512991eb8bc3e1c5f459c89...

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

Pages