Masterview devel - Nov 2006 - Fix for old directive impl compat framework in MV 0.3.0

Ed - would appreciate if you can confirm this fix solves your load problem.

I put in some additional unit tests to improve coverage in this area and
also did a quicky test in a rails app to load a directory with an old-impl
directive class to verify that it gets loaded and handled properly.  

(The new Directive Info page in the masterview Admin pages shows all the
loaded directives and namespaces, while Jeff''s handy Interactive Render
page
lets you test-drive directive markup)

And any feedback you have on this release would be much appreciated: did we
provide enough info in the release notes etc about what you as a directive
implementor had to do to hook into the backwards-compat framework?  And does
the new Developer docs we added in this release provide enough info for
someone to get started developing their own directive implementations or
does that need more work?

http://masterview.org/developer.html

Thanks!

~ Deb

On 11/12/06, Deb Lewis <djlewis at acm.org> wrote:

I found a couple of more problems. First the easy one. The docs say
that the image tag directive will take the wdith and height arguments,
if both are present and create a size="wxh" parameter. This
doesn''t
happen. You can try it in the interactive render page. It keeps both
widht and height.

The second one is tricky and I can''t figure it out. I have a <a>
tag
with an embedded img tag. I leave off the erb_content on the latter
and use content_string on the fomer. This will generate this
construct:

Original
<a href="#" wdt:link_to_remote_image=":url => {:action
=>
''hide_new''}"><img wdt:image_tag_link=""
src="../../../public/images/box_minimize.gif" /></a>
Output
<%= link_to_remote( image_tag( ''box_minimize.gif'' ), :url
=> {:action
=> ''hide_new''} ) %>
Which is what I want. But if I put any other attribute on the img tag,
it caues the combined
output to put the inner tag inside burly braces {}.

<a href="#" wdt:link_to_remote_image=":url => {:action
=>
''hide_new''}"><img wdt:image_tag_link=""
src="../../../public/images/box_minimize.gif" border="0" 
/></a>
Results in:
<%= link_to_remote( { image_tag( ''box_minimize.gif'',
:border => "0" )
}, :url => {:action => ''hide_new''} ) %>
Which causes the ActionController parse error to occur.
Now, the problem doesn''t seem to be in image_tag_link, because just by
itself, it looks ok:
<img wdt:image_tag_link=""
src="../../../public/images/box_minimize.gif" border="0" 
/>
Results in
image_tag( ''box_minimize.gif'', :border => "0" )

which is what I want.

This makes me suspect the call to method content_string in the
link_to_remote_image. If the content is a little complicated, does it
wrap the content in {}?

Ed
-- 
Ed Howland
http://greenprogrammer.blogspot.com

RE: mv:image_tag documented as mapping width/height attrs to :size parm for
rails helper

yep, looks like we lost that logic when the implementation was reworked; an
earlier version of the the directive did indeed pick up the dims attributes
and pass them on suitably to the rails helper.  Noted, will fix (?tomorrow?)

Your embedded directives case is trickier, I don''t have a quick answer
on
that one tonight, need to go back into the code and twist my brain back into
that scenario.  But certainly it ought to be the case that you get the same
result for the directive regardless of whether it''s embedded in a
containing
element which itself has a directive, so if that''s not happening
there''s a
bug somewhere.

~ Deb 

[soapbox rant: controlling border prop should be done in a stylesheet rule,
not by explicit presentational markup w/in doc content! but then your case
becomes needing to set, say, class attr on the <img> rather than border
prop, so your original problem report still needs to be addressed]

On 11/13/06, Deb Lewis <djlewis at acm.org> wrote:
> Your embedded directives case is trickier, I don''t have a quick
answer on
> that one tonight, need to go back into the code and twist my brain back
into
> that scenario.  But certainly it ought to be the case that you get the same
> result for the directive regardless of whether it''s embedded in a
containing
> element which itself has a directive, so if that''s not happening
there''s a
> bug somewhere.
Deb, I was able ro fix it by manipuating the various components of the
enclosing directive. I took the helper name I wanted (link_to_remote)
and added content_array[0].to_s and then appended any arguments (url,
etc.).

I am not certain, but the problem might be in prepare_output.

Ed

-- 
Ed Howland
http://greenprogrammer.blogspot.com

good, I''ll follow up and look into the content_string and
prepare_output
methods.

~ Deb 

-----Original Message-----
From: Ed Howland [mailto:ed.howland at gmail.com] 
Sent: Tuesday, November 14, 2006 7:30 AM
To: djlewis at acm.org
Cc: masterview-devel at rubyforge.org
Subject: Re: Fix for old directive impl compat framework in MV 0.3.0

On 11/13/06, Deb Lewis <djlewis at acm.org> wrote:
> Your embedded directives case is trickier, I don''t have a quick
answer
> on that one tonight, need to go back into the code and twist my brain 
> back into that scenario.  But certainly it ought to be the case that 
> you get the same result for the directive regardless of whether
it''s
> embedded in a containing element which itself has a directive, so if 
> that''s not happening there''s a bug somewhere.
Deb, I was able ro fix it by manipuating the various components of the
enclosing directive. I took the helper name I wanted (link_to_remote) and
added content_array[0].to_s and then appended any arguments (url, etc.).

I am not certain, but the problem might be in prepare_output.

Ed

--
Ed Howland
http://greenprogrammer.blogspot.com

On 11/13/06, Deb Lewis <djlewis at acm.org> wrote:
>
> RE: mv:image_tag documented as mapping width/height attrs to :size parm
> for
> rails helper
>
> yep, looks like we lost that logic when the implementation was reworked;
> an
> earlier version of the the directive did indeed pick up the dims
> attributes
> and pass them on suitably to the rails helper.  Noted, will fix
> (?tomorrow?)


Yes, the image_tag changed in this release. After looking at the code, I
realized that the image_tag you could either use :height and :width or the
:size attributes, so I went with the :width and :height instead of :size
since it seemed cleaner to me. I don''t really see why they even put the
size
option in there since it is unnecessary and height and width do what we
want.

Is there a reason you don''t want to use :width and :height? Last time I
looked at the code this should work identically to what :size was doing.

Jeff
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://rubyforge.org/pipermail/masterview-devel/attachments/20061115/49077caa/attachment.html

On 11/15/06, Jeff Barczewski <jeff.barczewski at gmail.com>
wrote:
>
> Yes, the image_tag changed in this release. After looking at the code, I
> realized that the image_tag you could either use :height and :width or the
> :size attributes, so I went with the :width and :height instead of :size
> since it seemed cleaner to me. I don''t really see why they even
put the size
> option in there since it is unnecessary and height and width do what we
> want.
>
> Is there a reason you don''t want to use :width and :height? Last
time I
> looked at the code this should work identically to what :size was doing.
>  Jeff
Jeff, yes, I guess you are right there. I thought the image_tag
required the :size parameter. But I guess that every (rails-)undefined
option in the hash gets translated into additional tag attributes.  I
didn''t realize that :size => ''XXxYY'' wasjust a
convenience parameter.

ed


-- 
Ed Howland
http://greenprogrammer.blogspot.com

On 11/15/06, Ed Howland <ed.howland at gmail.com>
wrote:
>
>
> Jeff, yes, I guess you are right there. I thought the image_tag
> required the :size parameter. But I guess that every (rails-)undefined
> option in the hash gets translated into additional tag attributes.  I
> didn''t realize that :size => ''XXxYY'' wasjust
a convenience parameter.
>
>

Also by using width and height separately you can choose to only set one or
the other (if you want, though I think this is a rare use case).

I apologize for not getting the docs updated regarding this tag, that was my
oversight.

Jeff
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://rubyforge.org/pipermail/masterview-devel/attachments/20061115/4bd9f67f/attachment.html