Commit ab3dc4957c1ad35112a90adef7cefb2ca9f84cd5
1 parent
d9254d1f
Exists in
master
and in
29 other branches
Enhancements in the Article lead feature
* Add Article#lead explicitly and moved the logic there
* Better UI for article
* Changed the the "short" format of blogs to use the lead intead of the
"first paragraph". With that the lead UI almost makes sense for all
users.
(ActionItem1664)
Showing
9 changed files
with
50 additions
and
9 deletions
Show diff stats
app/helpers/blog_helper.rb
| @@ -50,7 +50,7 @@ module BlogHelper | @@ -50,7 +50,7 @@ module BlogHelper | ||
| 50 | 50 | ||
| 51 | def display_short_format(article) | 51 | def display_short_format(article) |
| 52 | html = content_tag('div', | 52 | html = content_tag('div', |
| 53 | - article.first_paragraph + | 53 | + article.lead + |
| 54 | content_tag('div', | 54 | content_tag('div', |
| 55 | link_to_comments(article) + | 55 | link_to_comments(article) + |
| 56 | link_to( _('Read more'), article.url), | 56 | link_to( _('Read more'), article.url), |
app/models/article.rb
| @@ -345,6 +345,10 @@ class Article < ActiveRecord::Base | @@ -345,6 +345,10 @@ class Article < ActiveRecord::Base | ||
| 345 | Hpricot(to_html).search('p').first.to_html | 345 | Hpricot(to_html).search('p').first.to_html |
| 346 | end | 346 | end |
| 347 | 347 | ||
| 348 | + def lead | ||
| 349 | + abstract.blank? ? first_paragraph : abstract | ||
| 350 | + end | ||
| 351 | + | ||
| 348 | def creator | 352 | def creator |
| 349 | creator_id = versions[0][:last_changed_by_id] | 353 | creator_id = versions[0][:last_changed_by_id] |
| 350 | creator_id && Profile.find(creator_id) | 354 | creator_id && Profile.find(creator_id) |
app/models/published_article.rb
| @@ -32,4 +32,9 @@ class PublishedArticle < Article | @@ -32,4 +32,9 @@ class PublishedArticle < Article | ||
| 32 | def to_html(options={}) | 32 | def to_html(options={}) |
| 33 | reference_article ? reference_article.to_html : ('<em>' + _('The original text was removed.') + '</em>') | 33 | reference_article ? reference_article.to_html : ('<em>' + _('The original text was removed.') + '</em>') |
| 34 | end | 34 | end |
| 35 | + | ||
| 36 | + def abstract | ||
| 37 | + reference_article && reference_article.abstract | ||
| 38 | + end | ||
| 39 | + | ||
| 35 | end | 40 | end |
app/views/cms/_textile_article.rhtml
| @@ -5,7 +5,7 @@ | @@ -5,7 +5,7 @@ | ||
| 5 | <%= required labelled_form_field(_('Title'), text_field(:article, 'name', :size => '64')) %> | 5 | <%= required labelled_form_field(_('Title'), text_field(:article, 'name', :size => '64')) %> |
| 6 | 6 | ||
| 7 | <br style="clear: both;"/> | 7 | <br style="clear: both;"/> |
| 8 | -<%= button_to_function :edit, _("Lead"), nil, :id => "lead-link", :style => "margin-left: 0px;" %> | 8 | +<%= button :add, _("Lead"), '#', :id => "lead-button", :style => "margin-left: 0px;" %> |
| 9 | 9 | ||
| 10 | <div id="article-lead"> | 10 | <div id="article-lead"> |
| 11 | <%= labelled_form_field(_('Lead'), text_area(:article, 'abstract', :cols => 64, :rows => 10)) %> | 11 | <%= labelled_form_field(_('Lead'), text_area(:article, 'abstract', :cols => 64, :rows => 10)) %> |
app/views/cms/_tiny_mce_article.rhtml
| @@ -12,7 +12,7 @@ | @@ -12,7 +12,7 @@ | ||
| 12 | <% end %> | 12 | <% end %> |
| 13 | 13 | ||
| 14 | <br style="clear: both;"/> | 14 | <br style="clear: both;"/> |
| 15 | - <%= button_to_function :edit, _("Lead"), nil, :id => "lead-link", :style => "margin-left: 0px;" %> | 15 | + <%= button :add, _("Lead"), '#', :id => "lead-button", :style => "margin-left: 0px;" %> |
| 16 | 16 | ||
| 17 | <div id="article-lead"> | 17 | <div id="article-lead"> |
| 18 | <%= labelled_form_field(_('Lead'), text_area(:article, 'abstract', :style => 'width: 100%; height: 300px;')) %> | 18 | <%= labelled_form_field(_('Lead'), text_area(:article, 'abstract', :style => 'width: 100%; height: 300px;')) %> |
app/views/home/index.rhtml
| @@ -11,7 +11,7 @@ | @@ -11,7 +11,7 @@ | ||
| 11 | <div class='highlighted-news-item post-<%= index + 1 %>-inner'> | 11 | <div class='highlighted-news-item post-<%= index + 1 %>-inner'> |
| 12 | <h2><%= link_to(h(highlighted.title), highlighted.url, :class => 'post-title') %></h2> | 12 | <h2><%= link_to(h(highlighted.title), highlighted.url, :class => 'post-title') %></h2> |
| 13 | <span class="post-date"><%= show_date(highlighted.published_at, true) %> </span> | 13 | <span class="post-date"><%= show_date(highlighted.published_at, true) %> </span> |
| 14 | - <p class='headline'><%= !highlighted.abstract.blank? ? highlighted.abstract : highlighted.first_paragraph %></p> | 14 | + <div class='headline'><%= highlighted.lead %></div> |
| 15 | <p class='highlighted-news-read-more'> | 15 | <p class='highlighted-news-read-more'> |
| 16 | <%= link_to(_('Read more'), highlighted.url) %> | 16 | <%= link_to(_('Read more'), highlighted.url) %> |
| 17 | </p> | 17 | </p> |
public/javascripts/article.js
| 1 | (function($) { | 1 | (function($) { |
| 2 | - $("#lead-link").click(function(){ | ||
| 3 | - if($('#article-lead').css('display') == 'none') | ||
| 4 | - $('#article-lead').slideDown(); | ||
| 5 | - else | ||
| 6 | - $('#article-lead').slideUp(); | 2 | + $("#lead-button").click(function(){ |
| 3 | + $(this).toggleClass('icon-add').toggleClass('icon-remove'); | ||
| 4 | + $('#article-lead').slideToggle(); | ||
| 5 | + return false; | ||
| 7 | }) | 6 | }) |
| 8 | })(jQuery) | 7 | })(jQuery) |
test/unit/article_test.rb
| @@ -895,4 +895,22 @@ class ArticleTest < Test::Unit::TestCase | @@ -895,4 +895,22 @@ class ArticleTest < Test::Unit::TestCase | ||
| 895 | article.name = 'a123456789abcdefghij' | 895 | article.name = 'a123456789abcdefghij' |
| 896 | assert_equal 'a123456789ab...', article.short_title | 896 | assert_equal 'a123456789ab...', article.short_title |
| 897 | end | 897 | end |
| 898 | + | ||
| 899 | + should 'return abstract as lead' do | ||
| 900 | + a = Article.new(:abstract => 'lead') | ||
| 901 | + assert_equal 'lead', a.lead | ||
| 902 | + end | ||
| 903 | + | ||
| 904 | + should 'return first paragraph as lead by default' do | ||
| 905 | + a = Article.new | ||
| 906 | + a.stubs(:first_paragraph).returns('<p>first</p>') | ||
| 907 | + assert_equal '<p>first</p>', a.lead | ||
| 908 | + end | ||
| 909 | + | ||
| 910 | + should 'return first paragraph as lead with empty but non-null abstract' do | ||
| 911 | + a = Article.new(:abstract => '') | ||
| 912 | + a.stubs(:first_paragraph).returns('<p>first</p>') | ||
| 913 | + assert_equal '<p>first</p>', a.lead | ||
| 914 | + end | ||
| 915 | + | ||
| 898 | end | 916 | end |
test/unit/published_article_test.rb
| @@ -110,6 +110,21 @@ class PublishedArticleTest < ActiveSupport::TestCase | @@ -110,6 +110,21 @@ class PublishedArticleTest < ActiveSupport::TestCase | ||
| 110 | assert_match /removed/, p.to_html | 110 | assert_match /removed/, p.to_html |
| 111 | end | 111 | end |
| 112 | 112 | ||
| 113 | + should 'use abstract from referenced article' do | ||
| 114 | + original = Article.new(:abstract => 'this is the abstract') | ||
| 115 | + published = PublishedArticle.new | ||
| 116 | + published.stubs(:reference_article).returns(original) | ||
| 117 | + | ||
| 118 | + assert_equal 'this is the abstract', published.abstract | ||
| 119 | + end | ||
| 120 | + | ||
| 121 | + should 'return no abstract when reference_article does not exist' do | ||
| 122 | + published = PublishedArticle.new | ||
| 123 | + published.stubs(:reference_article).returns(nil) | ||
| 124 | + | ||
| 125 | + assert_nil published.abstract | ||
| 126 | + end | ||
| 127 | + | ||
| 113 | should 'specified parent overwrite blog' do | 128 | should 'specified parent overwrite blog' do |
| 114 | parent = mock | 129 | parent = mock |
| 115 | @article.stubs(:parent).returns(parent) | 130 | @article.stubs(:parent).returns(parent) |