Comment on xhtml in templates

esuomikim

Hi,

I'm just making several pages based on ZenPhoto. I just discovered ZenPhoto and I think it's great in it's simplicity.

But I have one comment. I'm a xhtml-strict fan and as for me it feels that lots of tags could be thrown of templates.

For example: `printDefaultSizedImage()` prints "width" and "height" tags while it could have been made in CSS file, and theming would be easier without those tags.

Another exaple: `printAdminToolbox()` prints a list of links with `
`'s in the end of lines, wouldn't it be better if it just printed a `

` list? The list is easier to theme, you can brake or position it's elements easier.

There are several other examples of tags that could have been replaced by CSS making the code clearer, IMHO.

acrylian

Well, generally zenphoto targets xhtml transitional. I think we can fight endless fights xhtml transitional vs strict vs html 4 (since w3 official choose html5 as the next and not xthml a while ago).

But I think you right with the admintoolbox list that should be a list.

The width and height for the <img> these is not used for (re)sizing! These are the actual values the image already has (which are done via php) and are only provided for browsers to have values to precalculate the space when generating the display of the page. This is recommend officially as far as I know, but I can be wrong.

esuomikim

No, I don't want to start xhtml wars : )

It was just a comment I thought could be helpful.

As for the the <img>, I know it's actual dimensions. I never used them. And I really don't remember that those were recommended, I know them as optional tags (W3Schools threats them like this also: http://www.w3schools.com/tags/tag_IMG.asp). In several different resources on web standards I also didn't seen anyone using those tags in xhtml...

I just think it is simpler to use as less html tags as possible and do all theming stuff in CSS, but that's me ; )

acrylian

I did understand your comment correctly..:-) I/we welcome any comment, suggestion or the like.

You are of course right that the width and height for images is not recommended and that they are optional (in opposite to alt for example) and probably the speed of precalculating the image dimensions is not that much. Well, maybe I just got stuck with a recommendation from older days when the net in general was slower...:-)

sbillard

Well, not all images fit the space provided. The width/height parameters are what is defined by the admin. They are there so if an image does not actually fit that format it will be resized by the browser to do so. Mostly this is for error images.

esuomikim

printAdminToolbox() printing Toolbox as a list, if one would like to replace it. It produces valid xhtml. `javascript.toogle(...)` link is thrown away, it could have be done in CSS.

`function printAdminToolbox($context=null, $id='admin') {

global $_zp_current_album, $_zp_current_image, $_zp_current_search, $_zp_loggedin, $_zen_gallery_page;

if (zp_loggedin()) {

$zf = WEBPATH."/".ZENFOLDER;

$dataid = $id . '_data';

$page = getCurrentPage();

$redirect = '';

echo "\n\n";

if (is_null($context)) { $context = get_context(); }

echo '

'."\n".'

'.gettext('Admin Toolbox').'

'."\n"."\n

";

echo '

'."\n";
echo "
• ";
  
  printAdminLink(gettext('Admin'), '', "\n");
  
  echo "
\n";
$albumname = $_zp_current_album->name;
if ($_zen_gallery_page === 'index.php') {
if (isMyAlbum($albumname, EDIT_RIGHTS)) {
echo "
• ";
  
  printSortableGalleryLink(gettext('Sort gallery'), gettext('Manual sorting'));
  
  echo "
\n";
}
if (isMyAlbum($albumname, UPLOAD_RIGHTS)) {
echo "
• ";
  
  printLink($zf . '/admin.php?page=upload' . urlencode($_zp_current_album->name), gettext("New album"), NULL, NULL, NULL);
  
  echo "
\n";
}
if (isset($_GET['p'])) {
$redirect = "&p=" . $_GET['p'];
}
if ($page>1) {
$redirect .= "&page=$page";
}
} else if ($_zen_gallery_page === 'album.php') {
if (isMyAlbum($albumname, EDIT_RIGHTS)) {
echo "
• ";
  
  printSubalbumAdmin(gettext('Edit album'), '', "
\n");
if (!$_zp_current_album->isDynamic()) {
echo "
• ";
  
  printSortableAlbumLink(gettext('Sort album'), gettext('Manual sorting'));
  
  echo "
\n";
}
echo "
• 
  urlencode($albumname) .
  
  "','".gettext("Are you sure you want to delete this entire album?")."','".gettext("Are you Absolutely Positively sure you want to delete the album? THIS CANNOT BE UNDONE!").
  
  "');\" title=\"".gettext("Delete the album")."\">".gettext("Delete album")."
\n";
}
if (isMyAlbum($albumname, UPLOAD_RIGHTS) && !$_zp_current_album->isDynamic()) {
echo "
• ";
  
  printLink($zf . '/admin.php?page=upload&album=' . urlencode($albumname), gettext("Upload Here"), NULL, NULL, NULL);
  
  echo "
\n";
echo "
• ";
  
  printLink($zf . '/admin.php?page=upload&new&album=' . urlencode($albumname), gettext("New Album Here"), NULL, NULL, NULL);
  
  echo "
\n";
}
$redirect = "&album=".urlencode($albumname)."&page=$page";
} else if ($_zen_gallery_page === 'image.php') {
$imagename = urlencode($_zp_current_image->filename);
if (isMyAlbum($albumname, EDIT_RIGHTS)) {
echo "
• 
  urlencode($albumname) . "&image=". urlencode($imagename) . "','". gettext("Are you sure you want to delete the image? THIS CANNOT BE UNDONE!") . "');\" title=\"".gettext("Delete the image")."\">".gettext("Delete image")."";
  
  echo "
\n";
}
$redirect = "&album=".urlencode($albumname)."&image=$imagename";
} else if (($_zen_gallery_page === 'search.php')&& !empty($_zp_current_search->words)) {
if ($_zp_loggedin & UPLOAD_RIGHTS) {
echo "
• ".gettext("Create Album")."
";
}
$redirect = "&p=search" . $_zp_current_search->getSearchParams() . "&page=$page";
}
echo "
• ".gettext("Logout")."
\n";
echo "

\n";

}

}`

acrylian

Thanks, we will take a look, I had it on the long list of things to do...:-)

Just to note: The best procedure for submitting fixes is always to create a ticket on trac and attach it the fix there. Also please always use the latest nightly/svn versio.