Thanks for clearing up some of those points. I have to disagree with you on a
couple of things though.
I would dearly love to ditch IE6 in favour of standard browsers but
unfortunately our users/clients don't (won't or can't)! I'll see what
happens though once we've implemented the view helper and styled it up and
browser checked in IE. If we come across any issues on our current project
with it, we'll extend the helper and make a feature request.
I personally feel that your coding with IF statements as assignments makes
it far less readable. In fact it was why I brought it up in the first place!
I was only skimming the surface of the code but it looked to be wrong.
Sometimes I will (and encourage others to) deliberately insert a couple of
extra lines of code just to make things more readable. When skimming though
code, one shouldn't have to do a double-take on a line to understand what it
does. But as long as it works I don't mind as the ZF code won't be touched
(or seen) by our developers so it doesn't really make a difference what is
happening behind the scenes. They just need to know how to use it.
I'd already written a custom view helper which does a similar thing to the
menu script anyway (generating a valid html nested list) which allows me the
flexibility of specifying my own classes/ids (it was written this way as it
was used not only for the navigation menu on the site). I'll have a play
around in the next few days seeing if I can modify it to just be an
extension of the menu helper.
Robin Skoglund wrote:
>
>
> SirEdward wrote:
>>
>> 1) The Menu helper automatically appends spaces to the output and these
>> are hard coded within the helper. While this means that the source looks
>> good when View Source is selected, we have a coding standard in place
>> where we use TABS for HTML output. Could this be changed to allow TABS as
>> well? The other View Helpers use ->setIndent() and I can use tabs in that
>> by calling ->setIndent("\t"). Any plans to do amend the Navigation view
>> helper to do this as well?
>>
>
> The setIndent() method is the same for all helpers, but for the menu
> helper it will only set the initial indentation for the menu. This will be
> fixed when Zend_View_Helper_Indent is added (it's currently a proposal).
>
>
> SirEdward wrote:
>>
>> 2) Also I seem to recall that previously I've had problems with the
>> content of li tags being on a separate line to the tags (probably in
>> IE6...) So could there be the option to output a tag as either -
>>
>> <li>Page 1</li>
>> or
>> <li>
>> Page 1
>> </li>
>>
>
> These are not the HTML tags you're looking for *jedi mind trick*.
>
> Seriously, though, I think the LI/IE6 thing is a non-issue, and we
> shouldn't drag down quality code to the "standards" of a browser that's 8
> years old. If you experience problems, you should post an issue to the
> issue tracker.
>
>
> SirEdward wrote:
>>
>> 3) I can't help but think there are a few errors in the code. I may be
>> totally wrong as I've only skimmed though, but I've come across bits like
>> this in Menu.php:
>>
>> Line #249
>> if ($this->getUseTranslator() && $t = $this->getTranslator()) {
>> Should it be
>> if ($this->getUseTranslator() && $t == $this->getTranslator()) {
>>
>> [...]
>>
>
> Those aren't errors, and the coding standards say nothing about
> assignments in if statements.
>
> Putting the assignment in the second condition makes the code more
> readable.
>
> The alternative is butt-ugly:
> if ($this->getUseTranslator() {
> $t = $this->getTranslator();
> if ($t) {
> // translate
> }
> }
>
> 3 lines extra, which are absolutely not needed. They only make the code
> harder to read/follow.
>
> Another alternative -- mentioned in another reply -- is this:
> $t = $this->getTranslator();
> if ($this->getUseTranslator() && $t) {
> // translate
> }
>
> ...which is even uglier and harder to read/follow, in addition to
> retrieving the translator even though it isn't used.
>
> So no, those aren't errors, and they won't be fixed. You should use the
> features of the language you're coding in (PHP), and write lean and
> aesthetically pleasing code.
>
>
> SirEdward wrote:
>>
>> 4) I see there is a CSS class of "active" which is added to a list item
>> when it's the "isActive()" page. However, I would like the option of
>> being able to supply my own class name(s). And the ability to add a CSS
>> class to other li tags as well - e.g. the first and last of a particular
>> list (including both selected and non selected lists), also classes to
>> odd and even tags in a list for Zebra crossing type effects. Plus I'd
>> like to add an ID to a top parent ul.
>>
>
> You have 3 options:
> 1) Use the features of CSS to achieve what you're asking
> 2) Use a partial view script (setPartial()/renderPartial())
> 3) Extend the Menu helper and do custom rendering
>
> robinsk
>
--
View this message in context:
http://www.nabble.com/Zend_Navigation-tp23048648p23068432.html
Sent from the Zend Framework mailing list archive at Nabble.com.
