Re: [Phpslash-devel] Re: [Phpslash-commit] CVS: phpslash-dev CHANGES,1.75,1.76

[Peter C. <li...@cr...>]

On Wed, 7 Jul 2004 14:13:25 -0500
Joe Stewart <joe...@us...> wrote:

> On Wed, Jul 07, 2004 at 10:56:13AM -0700, Peter Cruickshank wrote:
> 
> Thanks for the flood of commits.

There's on big one still waiting - the mod to the Story code to allow users
to pick an image to go with the article.

My code's at a pretty simple stage right now:
- articleimagedir/url optionally defined in config.ini.php, defaulting to
"$_PSL[imageurl]/articles"
- new table psl_story_image to hold the extra fields (and allow for
later potential of multiple images per article)
- The story edit screen gives a simple dropdown list of all images in the
articleimagedir with input of ALT text
- consequent changes to story.tpl and storyNew.tpl and the code that uses
them

NIce things I'd like to add at some point:
- a javascript-driven pop-up window to allow visual selection of images
- to be able to upload new images through the web interface (possibly using
the Back-End Upload module)
- separate thumbnail image

Any thoughts? Is this worth committing (it seems quite a hacky
approach to me). If so, should there be  an 'article.images' setting in
config.ini.php to turn this feature on and off, or would it be OK/better to
just have it in mainstream code?

It would be good to use the plugin feature to implement article images -
would it be possible?

> > + 2004-July 7 Peter Cruickshank <kr...@us...>
> > + [B]  slashTemplate.class:
> > +      Comment out portion of code that was causing infinite recursion
> > when attempting+      to find default skins template
> 
> Sorry you had to spend time figuring out what's going on there.
> 
> How did you trigger this?  A missing template file?  I haven't had a
> problem with it in months.  I think at one point I had it commented out
> as well.

I'd teaked stories/index.php to look for section-specific templates:

	$tpl = isset($ary['section_id']) ? 'storyIndex-'.$ary['section_id'] :
'storyIndex'; //PAC Hack for section templates
	$content   = $story->getStories($ary, $tpl);

The bug was hit when storyIndex-5.tpl (say) didnt exist.

> Any ideas on how to make this more simple while maintaining the
> flexibility?  The PEAR version of phplib templates creates an array of
> possible locations.  But if I remember it is doesn't allow on the fly
> changes like we have.

I'll have a look - I think the broad logic is OK.. Just to confirm the
search order:
- given skin, given language **
- current skin, current language
- parent skin, skin.parent.language (or current language)
- parent skin, default language
- "module directory"/templates/ ?? Why no language path??
- "module directory"
- default skin, default language* ?? Why not check current language first
??

* Is the check that was causing the loop
** This (new) check is required to prevent the loop, I think

I've not looked at the PEAR Template class....


> > + [B]  author/index.php:
> > +      Check for existance of jpcache_gc function before attempting to
> > call it
> 
> surely that is that last one.

That's what I thought the last time!

> > + [F]  Block_render_header.class
> > +      Make section name available to slashHead template. (There may be
> > a more elegant+      way of doing this)
> 
> yipes.  Looks like we weren't using the $extra_ary argument to getHeader.
>  I think 
> $_PSL['extra_ary'] can still be used though.  With these two arrays you
> can apply arbitrary text to be template variables.
> 
> We might want to change section->getName to generate a db object so it
> could be called like Section::getName.
> 
> So is the breadcrumb the page block not generating the section name?  Or
> is it not called before this?  This was subtly how the section name would
> be available before.  It would still need to be applied to extra_ary or
> pagetitle to be available in getHeader though.  This is one reason there
> hasn't been a breadcrumb block yet.  If breadcrumb isn't required, it
> probably should be made a block somehow.

I agree about making a breadcrumb block. Ideally, I would like to also make
the story title available to the head block. (So we could have 
<title>{SITE} :: {SECTION} {STORY}</title> when reading a story)

Maybe it is down to the order the blocks are being generated - since the
story title (if any) would only be known after most of the page has been
built. I must admit I'm still a little confused by what goes on with the
new page generation stuff, which isn't helping.

Related topic: do we really need to have $pagetitle and $relatedlinks as
global variables? It would seem nicer to put $pagetitle into $ary or $_PSL.
(Too late to give $ary a nicer name I guess). I kind of see why related
links has to be global, but it seems messy...

> > + [BW] Block_render_query.class
> > +      Fix generation of topic_id and section_id plus some minor tidyup
> > of logic+ [W]  Block_render_topic.class
> > +      Tidy up logic - incl move parse_str to top of parse()
> > + [W]  search/index.php section/admin.php topic/admin.php
> > +      Notice squashing
> > + [W]  Section.class, Topic.class
> > +      Improvements to logic + presentation of validation process
> > + 
> 
> >       // I am not sure if this check makes sense from the user's point
> >       of view// - it's a real pain deleting all the blocks one at a
> >       time.// Much better to just silently clear out
> >       psl_section_block_lut I think// - pac Jul-2004
> 
> I think I tend to agree, but it doesn't bother me too much the way it is
> either when deleting sections.  Don't tend to delete sections often.

I'll bring across (commented out) the lines to do the silent delete - so
they're  there as an option for developers.

-- 
Peter Cruickshank
pe...@cr...