Cool ... if you tested it and the MenuWidget, from my perspective it is good to go into master. And it's really great that you added the ScalaDocs !!!
Br's, Marius On Mar 23, 6:07 pm, Derek Chen-Becker <[email protected]> wrote: > OK, I've pushed the new code on the wip-dcb-dropdown branch. I made some > minor mods to the builtin Menu snippet (and changes to the Menu widget to > match): > > 1. Added an "expandAll" attribute that renders the entire SiteMap. > 2. Added a new "top" prefix to the builder snippet to allow for > attributes on the top-level menu UL element. > 3. Added API documentation to the Menu class. > > For some reason, the branch isn't showing up on github. I did a > > git push origin wip-dcb-dropdown > > And it says it's up-to-date, but I'm not seeing it on github. > > Derek > > On Sat, Mar 21, 2009 at 3:04 PM, Charles F. Munat <[email protected]> wrote: > > > > > +1 > > > I am understanding exactly what you have in mind and think it's a great > > idea. I wrote some kludgy code months ago (I knew zero about Scala back > > then -- now I know *infinity* times more! :-) to do pretty much the same > > thing, and I ended up completely rewriting the Menu snippet to my own > > needs. More flexibility == good, I think. > > > Chas. > > > (For anyone a little slow on the uptake this fine Saturday, please note > > that anything times zero is still zero.) > > > Derek Chen-Becker wrote: > > > I think I'm not being very clear. I'm going to modify the builtin Menu > > > snippet, not SiteMap. In my view, SiteMap should not be concerned with > > > how it's rendered, but the builtin snippet surely should be able to > > > handle this. Once I have the Menu snippet modified, the MenuWidget class > > > would just embed a <lift:Menu.builder expandAll="true" /> tag where it > > > wants the menu. I'll work on it this weekend and let you know when I > > > have something usable. > > > > Derek > > > > On Sat, Mar 21, 2009 at 10:01 AM, marius d. <[email protected] > > > <mailto:[email protected]>> wrote: > > > > I think I am aware what you meant ... If you want to make SiteMap to > > > know how to render itself as <ul>/<li> hence "centralize" this fine > > by > > > me. I have nothing against it, I just don't see the need ... the good > > > news is that it may be just me ;) > > > > The analogy with chooseTemplate is not very relevant IMHO because > > > choseeTemplate is a construct that one can use in many application > > > context whereas SiteMap is typically only one. But by all means don't > > > let me stay in your way:) > > > > If you are implementing this in SIteMap then perhaps you should make > > > both Menu built in snippet and MenuWidget to use it? > > > > Br's, > > > Marius > > > > On Mar 21, 5:41 pm, Derek Chen-Becker <[email protected] > > > <mailto:[email protected]>> wrote: > > > > Sorry, what I mean is that the builtin Menu snippet code is > > > duplicated, not > > > > SiteMap. For instance, here's how the builtin snippet generates > > > the ULs: > > > > > S.request.map(_.buildMenu.lines.toList match { > > > > case Nil => List(Text("No Navigation Defined.")) > > > > case xs => > > > > val liMap = S.prefixedAttrsToMap("li") > > > > val li = S.mapToAttrs(liMap) > > > > > def buildANavItem(i: MenuItem) = { > > > > i match { > > > > case MenuItem(text, uri, kids, true, _, _) => > > > > (<li><span>{text}</span>{buildUlLine(kids)}</li>) > > % > > > > S.prefixedAttrsToMetaData("li_item", liMap) > > > > case MenuItem(text, uri, kids, _, true, _) => > > > > (<li><a > > > href={uri}>{text}</a>{buildUlLine(kids)}</li>) % > > > > S.prefixedAttrsToMetaData("li_path", liMap) > > > > case MenuItem(text, uri, kids, _, _, _) => > > > > (<li><a > > > href={uri}>{text}</a>{buildUlLine(kids)}</li> % li) > > > > } > > > > } > > > > > def buildUlLine(in: Seq[MenuItem]): Node = if > > (in.isEmpty) > > > > Text("") > > > > else <ul>{in.flatMap(buildANavItem)}</ul> % > > > > S.prefixedAttrsToMetaData("ul") > > > > > buildUlLine(xs) > > > > > }) > > > > > And here's the new code you put together: > > > > > private def buildMenu(kids: Seq[MenuItem]): Elem = { > > > > <ul>{ > > > > for (m <- kids) yield { > > > > <li>{ > > > > <a href={m.uri}>{m.text}</a> ++ (m.kids.isEmpty match { > > > > case true => NodeSeq.Empty > > > > case _ => buildMenu(m.kids) > > > > }) > > > > }</li> > > > > } > > > > }</ul> > > > > } > > > > > What you've written is very similar, albeit without some of the > > > attribute > > > > handling, etc. I was just saying that if someone else comes along > > > and finds > > > > a need for fully expanding SiteMap into nested ULs, then they > > > have to write > > > > similar code again. It's a fairly small snippet of code, but I > > > don't know > > > > that that's really an argument against providing it as a general > > > mechanism. > > > > I mean, chooseTemplate is just a few lines, but we include it > > > because it's > > > > useful. I would be happy to make changes to the builtin Menu > > > snippet on a > > > > new branch to show what I mean if you'd like. > > > > > Derek > > > > > On Sat, Mar 21, 2009 at 8:22 AM, marius d. > > > <[email protected] <mailto:[email protected]>> wrote: > > > > > > Similar approach yes, duplicate not quite. It just utilizes > > > that the > > > > > API provided by SiteMap, MenuItem etc. and walks through a tree > > > > > structure and build <ul>/<li> constructs. > > > > > > We could make the SiteMap to be self aware of rendering itself > > > to <ul>/ > > > > > <li> constructs but personally I don't really see this as being > > > > > necessary. Nonetheless if there are compelling reasons to do so > > I'd > > > > > like to know them. ... I like things do be decoupled and SIteMap > > is > > > > > expressive enough to represent it in so many ways, > > > > > > Br's, > > > > > Marius > > > > > > On Mar 21, 3:51 pm, Derek Chen-Becker <[email protected] > > > <mailto:[email protected]>> wrote: > > > > > > Awesome! It looks like you did end up having to essentially > > > duplicate the > > > > > > code in the builtin Menu.builder snippet method to get this > > > working. When > > > > > I > > > > > > was looking at it I thought that it might be nice to have a > > > general way > > > > > of > > > > > > making the builtin Menu snippet just render the whole tree in > > > case anyone > > > > > > else wants to use it (for a site navigation page, etc). I was > > > thinking > > > > > that > > > > > > we could add an optional parameter that would essentially do > > > that, then > > > > > you > > > > > > could just have the snippet look like: > > > > > > > <lift:Menu.builder expandAll="true" /> > > > > > > > Or something. I suppose it's a pretty small chunk of code, > > > but if someone > > > > > > else wants to do something similar it means they have to copy > > > it all > > > > > over. > > > > > > > Thoughts? > > > > > > > Derek > > > > > > > On Sat, Mar 21, 2009 at 3:44 AM, Marius > > > <[email protected] <mailto:[email protected]>> wrote: > > > > > > > > Folks, > > > > > > > > I just committed the menu widget. Now the Widgets test site > > > uses this > > > > > > > widget instead of the traditional builtin menu. You can > > > also specify > > > > > > > the style of the menu using MenuStyle.HORIZONTAL, > > > MenuStyle.VERTICAL > > > > > > > and MenuStyle.NAVBAR. > > > > > > > > All these thanks to superfish jquery plugin > > > > > > >http://users.tpg.com.au/j_birch/plugins/superfish/ > > > > > > > > Oh you can also customize the superfish plugin by > > > specifying the JsObj > > > > > > > to set the properties. > > > > > > > > Thoughts/suggestions ? > > > > > > > > Br's, > > > > > > > Marius --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Lift" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/liftweb?hl=en -~----------~----~----~----~------~----~------~--~---
