From 6154889e8d52cee288e70bf5e93073a2b943ba5f Mon Sep 17 00:00:00 2001
From: joegatt <joe@joegatt.net>
Date: Tue, 27 Aug 2013 19:43:38 +0200
Subject: [PATCH] Show embeddable urls in notes lists and Link pages

---
 .../stylesheets/themes/default.css.sass       | 60 ++++++++++---------
 app/helpers/application_helper.rb             |  6 ++
 app/models/diffed_note_version.rb             |  7 ++-
 app/models/note.rb                            | 15 ++---
 app/views/application/_media.html.haml        |  3 +-
 app/views/application/_notes_list.html.haml   |  5 +-
 app/views/home/index.html.haml                |  2 +-
 app/views/links/_links_list.html.haml         |  2 +
 app/views/notes/show.html.haml                |  2 +-
 app/views/notes/version.html.haml             |  2 +-
 spec/helpers/application_helper_spec.rb       | 26 +++++---
 spec/models/diffed_note_version_spec.rb       |  2 +-
 spec/models/note_spec.rb                      | 12 ++--
 13 files changed, 82 insertions(+), 62 deletions(-)

diff --git a/app/assets/stylesheets/themes/default.css.sass b/app/assets/stylesheets/themes/default.css.sass
index f658d690..ae07dec8 100644
--- a/app/assets/stylesheets/themes/default.css.sass
+++ b/app/assets/stylesheets/themes/default.css.sass
@@ -182,7 +182,7 @@ iframe
   border: 0
   margin-bottom: $line-height
 
-img
+img, iframe
   display: block
   width: 100%
   max-width: 100%
@@ -357,6 +357,15 @@ figure
       &:before
         content: '\2014'
 
+iframe
+  &[src*='vimeo'],
+  &[src*='youtube']
+    height: $line-height * 8
+  &[src*='soundcloud.com']
+    height: $line-height * 2
+  &[src*='soundcloud.com%2Fplaylist']
+    height: $line-height * 8
+
 .annotations
   font:
     size: $small-font-size + 1px
@@ -503,16 +512,6 @@ figure
     cite
       display: none
 
-.home-index
-  iframe
-    &[src*='vimeo'],
-    &[src*='youtube']
-      height: $line-height * 25
-    &[src*='soundcloud.com']
-      height: $line-height * 6
-    &[src*='soundcloud.com%2Fplaylist']
-      height: $line-height * 15
-
 .links-show_channel .links
   & > ul
     margin-bottom: 0
@@ -570,7 +569,7 @@ figure
 
   +pass_media_query('print')
 
-  nav
+  iframe, nav
     display: none
 
   a
@@ -626,6 +625,17 @@ figure
     height: $line-height * 2
     margin: $line-height 0
 
+  iframe
+    +column(12, 12)
+    +first-column
+    &[src*='vimeo'],
+    &[src*='youtube']
+      height: $line-height * 16
+    &[src*='soundcloud.com']
+      height: $line-height * 6
+    &[src*='soundcloud.com%2Fplaylist']
+      height: $line-height * 15
+
   .citations
     +column(8, 12)
     +pull(4, 12)
@@ -639,11 +649,12 @@ figure
       a
         padding-right: 34.38889% // OPTIMIZE: We get these measurements from the columns.
         +blurb(5, 34.38889%)     //  Ideally, we would sue the semantic.gs algorithm.
-      img
+      img, iframe
+        height: $line-height * 5 // Use variable
+        margin: 0
         position: absolute
-        width: auto
         right: 0
-        height: $line-height * 5 // Use variable
+        width: auto
 
   .books-edit, .books-update, .links-edit, .links-update
     form
@@ -680,15 +691,6 @@ figure
           position: absolute
           height: $line-height * 6
 
-    iframe
-      &[src*='vimeo'],
-      &[src*='youtube']
-        height: $line-height * 16
-      &[src*='soundcloud.com']
-        height: $line-height * 6
-      &[src*='soundcloud.com%2Fplaylist']
-        height: $line-height * 15
-
     section#content
       #text
         +clearfix
@@ -790,6 +792,11 @@ figure
 
   +pass_media_query('screen-and-min-width-1024px')
 
+  iframe
+    +column(9, 12)
+    +first-column
+    +pull(3, 12)
+
   img
     &[src*='16-9-8']
       height: 390px
@@ -809,7 +816,7 @@ figure
     li
       a
         +blurb(5, 34.38889%)
-      img
+      img, iframe
         width: 240px // OPTIMIZE: We get these measurements from the columns.
         height: $line-height * 5
 
@@ -843,9 +850,6 @@ figure
           +first-column
         #map
           +column(3, 12)
-      iframe
-        +column(9, 12)
-
       #text
         .body
           +column(6, 12)
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 7b41f12e..0e4a6014 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -22,6 +22,12 @@ def current_url
     "#{ request.protocol }#{ request.host_with_port }#{ request.fullpath }"
   end
 
+  def embeddable_url(url)
+    url.gsub(/^.*youtube.*v=(.*)\b/, 'http://www.youtube.com/embed/\\1?rel=0')
+       .gsub(%r(^.*vimeo.*/(\d*)\b), 'http://player.vimeo.com/video/\\1')
+       .gsub(/(^.*soundcloud.*$)/, 'http://w.soundcloud.com/player/?url=\\1')
+  end
+
   def qr_code_image_url(size = Settings.styling.qr_code_image_size)
     "https://chart.googleapis.com/chart?chs=#{ size }x#{ size }&cht=qr&chl=#{ current_url }"
   end
diff --git a/app/models/diffed_note_version.rb b/app/models/diffed_note_version.rb
index 8bcfff6c..e41a632d 100644
--- a/app/models/diffed_note_version.rb
+++ b/app/models/diffed_note_version.rb
@@ -2,8 +2,8 @@
 
 class DiffedNoteVersion
 
-  attr_accessor :sequence, :title, :body, :tag_list, :previous_body, :previous_title, :previous_tag_list,
-                :embeddable_source_url, :external_updated_at
+  attr_accessor :sequence, :title, :body, :tag_list, :previous_body, :previous_title, :previous_tag_list, 
+                :is_embeddable_source_url, :external_updated_at
 
   def initialize(note, sequence)
 
@@ -34,7 +34,8 @@ def initialize(note, sequence)
     self.sequence = sequence
     self.title = version.title
     self.body = version.body
-    self.embeddable_source_url = version.embeddable_source_url
+    self.is_embeddable_source_url = version.is_embeddable_source_url
     self.external_updated_at = version.external_updated_at
   end
+
 end
diff --git a/app/models/note.rb b/app/models/note.rb
index 265b5959..94ce199a 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -87,15 +87,10 @@ def clean_body
       .gsub(/\s+/, ' ')
   end
 
-  def embeddable_source_url
-    if source_url && source_url =~ /youtube|vimeo|soundcloud/
-      source_url
-        .gsub(/^.*youtube.*v=(.*)\b/, 'http://www.youtube.com/embed/\\1?rel=0')
-        .gsub(%r(^.*vimeo.*/video/(\d*)\b), 'http://player.vimeo.com/video/\\1')
-        .gsub(/(^.*soundcloud.*$)/, 'http://w.soundcloud.com/player/?url=\\1')
-    else
-      nil
-    end
+  # REVIEW: If we named this embeddable_source_url? then we can't do 
+  #  self.embeddable_source_url? = version.embeddable_source_url? in diffed_version
+  def is_embeddable_source_url
+    (source_url && source_url =~ /youtube|vimeo|soundcloud/)
   end
 
   def fx
@@ -140,7 +135,7 @@ def scan_note_for_urls
   end
 
   def body_or_source_or_resource?
-    if body.blank? && embeddable_source_url.blank? && resources.blank?
+    if body.blank? && !is_embeddable_source_url && resources.blank?
       errors.add(:note, 'Note needs one of body, source or resource.')
     end
   end
diff --git a/app/views/application/_media.html.haml b/app/views/application/_media.html.haml
index 5b927ae3..d2a11c21 100644
--- a/app/views/application/_media.html.haml
+++ b/app/views/application/_media.html.haml
@@ -1,2 +1 @@
-- if note.embeddable_source_url =~ /youtube|vimeo|soundcloud/
-  %iframe{ :src => note.embeddable_source_url, :allowfullscreen => 'true', :webkitAllowFullScreen => 'true', :webkitAllowFullScreen => 'true' }
+%iframe{ src: embeddable_url(url), allowfullscreen: 'true', webkitAllowFullScreen: 'true', webkitAllowFullScreen: 'true' }
diff --git a/app/views/application/_notes_list.html.haml b/app/views/application/_notes_list.html.haml
index 68d4acd4..ebc349a5 100644
--- a/app/views/application/_notes_list.html.haml
+++ b/app/views/application/_notes_list.html.haml
@@ -11,6 +11,9 @@
       - image = note.resources.attached_images.first
 
       %a{ href: note_path(note), lang: lang_attr(note.lang), dir: dir_attr(note.lang) }
-        = image_tag(cut_image_binary_path(image, type: :thumbnail), alt: strip_tags(image.description)) unless image.nil?
+        - if image
+          = image_tag(cut_image_binary_path(image, type: :thumbnail), alt: strip_tags(image.description))
+        - elsif note.is_embeddable_source_url
+          = render 'media', url: note.source_url
         %h2= blurbify(headline, note.books, note.links) 
         ~ blurbify(blurb, note.books, note.links)
diff --git a/app/views/home/index.html.haml b/app/views/home/index.html.haml
index 951d8d16..7d275b62 100644
--- a/app/views/home/index.html.haml
+++ b/app/views/home/index.html.haml
@@ -15,7 +15,7 @@
     - if image.caption
       %figcaption= image.caption.html_safe
 
-  = render 'media', note: @notes.first
+  = render 'media', url: @notes.first.source_url if @note.is_embeddable_source_url
 
 - elsif @notes.size > 1
 
diff --git a/app/views/links/_links_list.html.haml b/app/views/links/_links_list.html.haml
index 37ca94e3..d2a598f3 100644
--- a/app/views/links/_links_list.html.haml
+++ b/app/views/links/_links_list.html.haml
@@ -6,6 +6,8 @@
   %ul
     = list_of links do |link|
 
+      = render 'media', url: embeddable_url(link.url) if link.url =~ /youtube|vimeo|soundcloud/
+
       %cite= link_to link.title, link.url
       = t('citation.link.accessed_at_html', accessed_at: (timeago_tag link.updated_at))
       %p= link_to link.url.gsub(%r(https?://), ''), link.url, class: 'url'
diff --git a/app/views/notes/show.html.haml b/app/views/notes/show.html.haml
index ca4fd599..082eedee 100644
--- a/app/views/notes/show.html.haml
+++ b/app/views/notes/show.html.haml
@@ -11,7 +11,7 @@
 
     = render 'images', images: @note.resources.attached_images, type: :standard unless @note.resources.attached_images.empty?
 
-    = render 'media', note: @note unless @note.embeddable_source_url.blank?
+    = render 'media', url: @note.source_url if @note.is_embeddable_source_url
 
     %section#text
       = bodify(@note.body, @note.books, @note.links)
diff --git a/app/views/notes/version.html.haml b/app/views/notes/version.html.haml
index 0bec3e3a..df57f90b 100644
--- a/app/views/notes/version.html.haml
+++ b/app/views/notes/version.html.haml
@@ -13,7 +13,7 @@
 
     = render 'header', title: title, document_title: document_title
 
-    = render 'media', note: @diffed_version
+    = render 'media', url: @diffed_version.source_url if @diffed_version.is_embeddable_source_url
 
     -#    %section#text!= bodify(Differ.diff_by_word(@diffed_version.body, @diffed_version.previous_body))
     %section#text!= Differ.diff_by_word(bodify(@diffed_version.body), bodify(@diffed_version.previous_body))
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 1543b721..05ba4d96 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -5,20 +5,20 @@
   describe '#lang_attr' do
     before { I18n.locale = 'en' }
     it 'should return the language if different from locale' do
-      lang_attr('ar').should == 'ar'
+      lang_attr('ar').should eq('ar')
     end
     it 'should return nil if the language is same as locale' do
-      lang_attr('en').should == nil
+      lang_attr('en').should be_nil
     end
   end
 
   describe '#body_dir_attr' do
     before { Settings.lang['rtl_langs'] = ['ar'] }
     it 'should return "rtl" if language is rtl' do
-      body_dir_attr('ar').should == 'rtl'
+      body_dir_attr('ar').should eq('rtl')
     end
     it 'should return "ltr" if language is not rtl' do
-      body_dir_attr('en').should == 'ltr'
+      body_dir_attr('en').should eq('ltr')
     end
   end
 
@@ -29,10 +29,10 @@
         I18n.locale = 'ar'
       end
       it 'returns "ltr" if language is not the same as locale, and is ltr' do
-        dir_attr('en').should == 'ltr'
+        dir_attr('en').should eq('ltr')
       end
       it 'returns nil if language is the same as locale, and is rtl' do
-        dir_attr('ar').should == nil
+        dir_attr('ar').should be_nil
       end
     end
     context 'when the note is not in the default language' do
@@ -41,11 +41,21 @@
         I18n.locale = 'en'
       end
       it 'returns nil if language is the same as locale' do
-        dir_attr('en').should == nil
+        dir_attr('en').should be_nil
       end
       it 'returns "rtl" if language is not the same as locale, and is rtl' do
-        dir_attr('ar').should == 'rtl'
+        dir_attr('ar').should eq('rtl')
       end
     end
   end
+
+  describe '#embeddable_url' do
+    specify { embeddable_url('http://www.example.com').should eq('http://www.example.com') }
+    specify { embeddable_url('http://youtube.com?v=ABCDEF').should eq('http://www.youtube.com/embed/ABCDEF?rel=0') }
+    specify { embeddable_url('http://vimeo.com/video/ABCDEF').should eq('http://player.vimeo.com/video/ABCDEF') }
+    specify { embeddable_url('http://vimeo.com/ABCDEF').should eq('http://player.vimeo.com/video/ABCDEF') }
+    specify { embeddable_url('http://soundcloud.com?v=ABCDEF')
+              .should eq('http://w.soundcloud.com/player/?url=http://soundcloud.com?v=ABCDEF') }
+  end
+
 end
diff --git a/spec/models/diffed_note_version_spec.rb b/spec/models/diffed_note_version_spec.rb
index 99a84339..7933267f 100644
--- a/spec/models/diffed_note_version_spec.rb
+++ b/spec/models/diffed_note_version_spec.rb
@@ -18,7 +18,7 @@
   it { should respond_to(:previous_title) }
   it { should respond_to(:previous_body) }
   it { should respond_to(:previous_tag_list) }
-  it { should respond_to(:embeddable_source_url) }
+  it { should respond_to(:is_embeddable_source_url) }
   it { should respond_to(:external_updated_at) }
 
   context 'when the first version is requested' do
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 63ab01c4..ea05176a 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -18,7 +18,7 @@
   it { should respond_to(:source_url) }
   it { should respond_to(:source_application) }
   it { should respond_to(:last_edited_by) }
-  it { should respond_to(:embeddable_source_url) }
+  it { should respond_to(:is_embeddable_source_url) }
   it { should respond_to(:fx) }
   it { should respond_to(:active) }
   it { should respond_to(:hide) }
@@ -268,25 +268,25 @@
     pending 'TODO'
   end
 
-  describe '#embeddable_source_url' do
+  describe '#is_embeddable_source_url' do
     context 'when source_url is not known to be embeddable' do
       before { note.source_url = 'http://www.example.com' }
-      its(:embeddable_source_url) { should be_nil }
+      its(:is_embeddable_source_url) { should be_false }
     end
 
     context 'when source_url is a youtube link' do
       before { note.source_url = 'http://youtube.com?v=ABCDEF' }
-      its(:embeddable_source_url) { should == 'http://www.youtube.com/embed/ABCDEF?rel=0' }
+      its(:is_embeddable_source_url) { should be_true }
     end
 
     context 'when source_url is a vimeo link' do
       before { note.source_url = 'http://vimeo.com/video/ABCDEF' }
-      its(:embeddable_source_url) { should == 'http://player.vimeo.com/video/ABCDEF' }
+      its(:is_embeddable_source_url) { should be_true }
     end
 
     context 'when source_url is a soundcloud link' do
       before { note.source_url = 'http://soundcloud.com?v=ABCDEF' }
-      its (:embeddable_source_url) { should == 'http://w.soundcloud.com/player/?url=http://soundcloud.com?v=ABCDEF' }
+      its (:is_embeddable_source_url) { should be_true }
     end
   end