we could have

[4:20 PM] and there is no reason to have theme_links() AND theme_item_list() [4:20 PM] they should be combined. [4:21 PM] theme_list() [4:21 PM] hmm, I commented on that issue [4:21 PM] yoroy joined the channel [4:21 PM] don't actually remember [4:21 PM] yeah it was a while back. [4:21 PM] aspilicious: does that make sense? [4:22 PM] I'm confused what the other issue is [4:22 PM] aspilicious his brain is dead after a day of studying [4:22 PM] ericduran: which other issue? [4:22 PM] aspilicious: LOL [4:22 PM] sviebrock_ joined the channel [4:22 PM] jacine, where do we add the div actually? [4:22 PM] jacine: regarding theme_links and theme_item_list [4:22 PM] aspilicious: in theme_item_list() the div is added in the theme function. [4:23 PM] ow on the bottom [4:23 PM] ericduran: you are confused about why we don't need both? [4:23 PM] It's kinda dumb actually. [4:23 PM] I'm confused what issue this is [4:23 PM] lol [4:23 PM] I'm just confused! [4:24 PM] jacine, in theme_links where do we add the surrounding div? [4:24 PM] LOL. Well, we were looking at this: http://drupal.org/node/1190210 [4:24 PM] http://drupal.org/node/1190210 => Convert toolbar.tpl.php to HTML5 => Drupal core, theme system, normal, needs review, 47 comments, 7 IRC mentions [4:24 PM] aspilicious: theme_links() we don't do that. [4:24 PM] hmm [4:24 PM] just a heading that floats around aimlessly [4:24 PM] there we go, thanks [4:24 PM] ;) [4:25 PM] Cottser quit IRC (Remote host closed the connection) [4:25 PM] I was saying I think theme_nav() is a bad idea. [4:25 PM] and now I see the connection [4:25 PM] :) [4:25 PM] yea, theme_nav can be theme_links with a type [4:25 PM] right [4:25 PM] and i was saying it should just be theme_list() [4:26 PM] and that theme_links() and theme_item_list() should be combined. [4:26 PM] does anyone has a drupal core open in his editor? [4:26 PM] because it's dumb to have both. [4:26 PM] aspilicious: why what should we look at? [4:26 PM] aspilicious: I have it available in a command [4:27 PM] jacine, ow ok it's ok [4:27 PM] so if we are going to go with the 3 nav structure [4:27 PM] so are we going to combine them? [4:27 PM] we are going to add another level [4:28 PM] are you ok with that? [4:28 PM] ericduran: I was waiting until some changes to both theme functions got committed, but they have so we really just need to create a new issue. [4:28 PM] we should add the type parameter to theme links 1st [4:28 PM] then combine them [4:28 PM] or we go the easy but less accurate way and just replace the surrounding div with nav [4:28 PM] it should be simpler ideally [4:29 PM] aspilicious: I think that 1 nav is probably enough, with 3 lists in it. [4:29 PM] jacine so you don't care about the toggle and stuff [4:29 PM] aspilicious: but, I'd like to see more feedback from others. [4:29 PM] aspilicious: I think they get the same semantic meaning if they are all in a nav. [4:30 PM] can I suggest that we see what the final HTML looks like and compare? [4:30 PM] MD3: that is what the patch does. [4:31 PM] MD3: do you know how to apply patches? [4:31 PM] jacine, http://paste.pocoo.org/show/540209/ [4:31 PM] jacine: no. I would love to learn, but that sounds like something for after the meeting? [4:31 PM] like the top or the bottom? [4:31 PM] (it's a quick sketch) [4:32 PM] When I say, we need feedback, I mean people need to review the existing patch and comment with feedback. That's how it works. Here is a tutorial on patches: http://jacine.net/post/8419331209/patches [4:32 PM] glennpratt quit IRC (Remote host closed the connection) [4:32 PM] aspilicious: I think the first one. [4:32 PM] aspilicious: needs the aria role="navigation" tho ;) [4:32 PM] ok there is a patch that does that [4:33 PM] jacine, so we can rtbc that if we want :p [4:33 PM] cool [4:33 PM] http://drupal.org/node/1190210#comment-5462870 [4:33 PM] http://drupal.org/node/1190210 => Convert toolbar.tpl.php to HTML5 => Drupal core, theme system, normal, needs review, 45 comments, 8 IRC mentions [4:33 PM] jacine: thanks. I'll add that to my reading list for tonight [4:33 PM] MD3: np [4:34 PM] attiks quit IRC (Ping timeout: 240 seconds) [4:34 PM] aspilicious: that is a much simpler patch [4:34 PM] ericduran, yeah it just replaces the div with nav [4:34 PM] aspilicious: can you post it again at the bottom though so we don't confuse people. [4:34 PM] yea [4:34 PM] and everet said it was ok [4:34 PM] yeah, that looks much better. [4:35 PM] aspilicious: well that patch needs a small change we need to add the navigation role [4:35 PM] btw I would like http://drupal.org/node/1190210#comment-5462870 to be part of this or the next sprint. It's done except for test problems my brain can't fix [4:35 PM] http://drupal.org/node/1190210 => Convert toolbar.tpl.php to HTML5 => Drupal core, theme system, normal, needs review, 47 comments, 9 IRC mentions [4:35 PM] aspilicious: the last patch have the navigation role, that previous patch does not [4:35 PM] aspilicious: I already tagged it for you ;) [4:35 PM] ericduran, +