Friday, September 18, 2009

Minor Details, Part 1

‹prev | My Chain | next›

Clicking about the site, I notice a few minor problems. Mostly the little details that get added to a site after years of development. Most of them can be seen on this recipe screenshot:



On that page:
  • there is no title on the browser tab
  • there is no footer on the page
  • recipes do not include links at the bottom to search Google for similar recipes
  • recipes do not include links at the bottom to search Amazon for cookbooks that might contain similar recipes
I can live with most of those issues. More bothersome to me are problems with the ingredient list:



The list of ingredients:
  • has no heading
  • does not "wiki-fy" links (e.g. [recipe:2002/02/15/cocktail_sauce cocktail sauce] )
  • has decimal numbers instead of pretty fractions (¼, ½, ¾, etc.)
  • can use some additional CSS love—brand name (e.g. Honey Maid) should be de-emphasized, the text should not be quite so close to the "Servings and Times" and "Tools and Appliances" info boxes.
The first item on the list is easy enough to address—I add this to the recipe.haml template:
...
- if @recipe['preparations']
%h2= "Ingredients"
%ul.preparations
- @recipe['preparations'].each do |preparation|
%li.ingredient
...
I am not altering any behavior of the template nor am I affecting the structure of the resulting DOM. It is just an additional label. In other words, no tests needed.

Evaluating the recipe wiki text does describe a change in behavior, so off to my specs! I only want to ensure that the recipe name is wiki-fied, so a simple :should_receive ought to suffice in an RSpec example:
    it "should wiki-fy the link to the other recipe" do
self.should_receive(:wiki).
with("[recipe:2009/09/18/recipe Recipe]").
and_return(%Q|<a href="http://example.org/">Bar</a>|)

render("views/recipe.haml")
end
I make that pass by simply adding a wiki helper call to the ingredient name attribute:
...
- if @recipe['preparations']
%h2= "Ingredients"
%ul.preparations
- @recipe['preparations'].each do |preparation|
%li.ingredient
%span.quantity
= preparation['quantity']
%span.unit
= preparation['unit']
%span.kind
= preparation['ingredient']['kind']
%span.name
= wiki preparation['ingredient']['name']
- if preparation['brand'] =~ /\S/
%span.brand
= "(" + preparation['brand'] + ")"
...
That gets the spec passing. It definitely includes the link on the page now, but inside a <p> tag, which throws off the display:



That extraneous <p> tag is being added by the call to RedCloth in the wiki helper:
    def wiki(original)
text = (original || '').dup
text.gsub!(/\b(\d+)F/, "\\1° F")
text.gsub!(/\[kid:(\w+)\]/m) { |kid| kid_nicknames[$1] }
text.gsub!(/\[recipe:(\S+)\]/m) { |r| recipe_link($1) }
text.gsub!(/\[recipe:(\S+)\s(.+?)\]/m) { |r| recipe_link($1, $2) }
text.gsub!(/\[meal:(\S+)\]/m) { |m| meal_link($1) }
text.gsub!(/\[meal:(\S+)\s(.+?)\]/m) { |m| meal_link($1, $2) }
RedCloth.new(text).to_html
end
There is no need for RedCloth in this case, it would be nice to be able to tell wiki to not use it:
  it "should skip converting textile to HTML if arg2 is false" do
wiki("textile", false).
should_not have_selector("p")
end
That fails with the following error:
cstrom@jaynestown:~/repos/eee-code$ spec ./spec/eee_helpers_spec.rb 
...........F............................................................................

1)
ArgumentError in 'wiki should skip converting textile to HTML if arg2 is false'
wrong number of arguments (2 for 1)
./spec/eee_helpers_spec.rb:67:in `wiki'
./spec/eee_helpers_spec.rb:67:

Finished in 0.149237 seconds

88 examples, 1 failure
I change the message by adding an optional second argument to the wiki helper method:
    def wiki(original, convert_textile=true)
...
RedCloth.new(text).to_html
end
The spec still fails because I am still converting textile with RedCloth:
cstrom@jaynestown:~/repos/eee-code$ spec ./spec/eee_helpers_spec.rb 
...........F............................................................................

1)
'wiki should skip converting textile to HTML if arg2 is false' FAILED
expected following output to omit a <p/>:
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN" "http://www.w3.org/TR/REC-html40/loose.dtd">
<html><body><p>textile>/p></body></html>
./spec/eee_helpers_spec.rb:67:

Finished in 0.154897 seconds

88 examples, 1 failure
To make that pass, I use the second argument in a ternary to by-pass the RedCloth call:
    def wiki(original, convert_textile=true)
...
convert_textile ? RedCloth.new(text).to_html : text
end
That makes it pass, except that it doesn't pass:
cstrom@jaynestown:~/repos/eee-code$ spec ./spec/eee_helpers_spec.rb 
...........F............................................................................

1)
'wiki should skip converting textile to HTML if arg2 is false' FAILED
expected following output to omit a <p/>:
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN" "http://www.w3.org/TR/REC-html40/loose.dtd">
<html><body><p>textile>/p></body></html>
./spec/eee_helpers_spec.rb:67:

Finished in 0.154897 seconds

88 examples, 1 failure
Hunh? I changed that, how could it possibly be failing? It definitely works in the browser now:



So what gives?

It turns out to be added by RSpec itself. To work around, I change the expectation from omitting the <p/> tag to not invoking RedCloth. That is more implementation specific, but I am not too concerned—after all, I really am trying to avoid the RedCloth usage here:
  it "should skip converting textile to HTML if arg2 is false" do
RedCloth.should_not_receive(:new)
wiki("textile", false)
end
With specs passing and display looking good, I call it a day at this point. I will pick up tomorrow with the prettifying of the decimal numbers in the ingredient quantities.

No comments:

Post a Comment