search

dominickm / jupiter_broadcasting_mobile_community

The Jupiter Broadcasting community project.
Other
100 stars 17 forks source link

Pull for Series Page #13

Closed ograycode closed 11 years ago

ograycode commented 11 years ago

Commit:

Added series page, rss, credits.md and formatted index.html 

Series page now pulls from JB's OGG rss feed. It is bare-bones and designed to be built off of.

I also added credits.md to keep track of what projects are used, so credit can be given where credit is due.

If you have any questions or concerns, feel free to let me know.

Also, if I want to work on something else, that is separate from this, do I just commit back to my clone and send another pull request, and if this one gets rejected the other can be cherry picked? Haven't done to much github stuff before.

ghost commented 11 years ago

I have just merged your changes into my fork and tested it. It works, which is really cool, however there is just one slight issue.

My Intention for the series filter was that you could filter through every episode in a particular series. The "all" filter is for the latest shows. Perhaps we need to change the filter "all" to "latest" as it seems a little misleading. When I was creating the header for the shows page I was trying to imagine how users would want to get their shows.

My concept included creating a filter for:

It should have included this in my commit comment. So I apologise, since I was making assumptions that everyone would understand what I ment. If you wouldn't mind, Would you please change the "all" to "latest" in index.html and change the series object in jquery.jb.js so that it places the feed into "latest"?

ograycode commented 11 years ago

Ya, all I really have to do is change the div and the click. I'll have another commit up later today.

ghost commented 11 years ago

I also noticed you place the script after the /body tag I'm not sure that you should do that...It is not something you see. You really should use put it in the head tag or in an external file. What you need to use onload (JavaScript) or $(document).ready instead.

Not to nit pick. Please check your spelling before commiting. I noticed that the contribute file has a typo.

ograycode commented 11 years ago

The script is after the body tag for loading performance reasons, and we should actually be moving all scripts to the bottom, if at all possible, see http://developer.yahoo.com/performance/rules.html#js_bottom

$(function(){}); is the equivalent of $(document).ready, see http://api.jquery.com/ready/

ghost commented 11 years ago

That's Interesting. From what I have goggled the issue was more about SEO than parallel download optimisation. It came from a Microsoft presentation that discussed how meta search crawlers only look through x amount of data. It is true about parallel downloading, though I don't know if we will benefit from this. It works well so I'm not suggesting to change it.

As for the $(document).ready - I clearly didn't realise that it was the same thing. So thanks for both references: I think I have learnt something today.

Do you think we should use an external sheet for the bottom of this page? One benefit would be that it may make git commits a little cleaner.

I think for now the jQuery Mobile UX scripts (aka the stuff I did) should stay up the top, as the bottom may prevent document.write. I am concerned that this could mess up the UX. The JB namespace could utilise the jQuery library instead of document.write. So we could place it at the bottom as long as it does not break anything. If we do move it down the bottom, then I would like to suggest that we include your code inside the file. We are going to need a $(document).ready function anyway.

ghost commented 11 years ago

Actually all of the JavaScript could go at the bottom. It works.

mfriesen commented 11 years ago

Your comment via Code Journal