From 76c0c7f3a1246319242e8fff7649c1450da0f98e Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 27 Oct 2009 13:46:24 -0700
Subject: Change our menu building blocks to use PHP templates so that themes
 can override them and define their own menu formats.  I worry a little bit
 that this approach may be too heavy since we're now doing a lot more template
 includes than we were before.  Also, I had to change the Menu API to stop
 using __toString() because you can't throw exceptions from __toString() which
 would make it an unhappy experience for developers.

---
 modules/gallery/libraries/Admin_View.php      |  3 +-
 modules/gallery/libraries/Menu.php            | 61 +++++++--------------------
 modules/gallery/libraries/Theme_View.php      | 12 +++---
 modules/gallery/views/menu.html.php           | 23 ++++++++++
 modules/gallery/views/menu_ajax_link.html.php | 10 +++++
 modules/gallery/views/menu_dialog.html.php    |  9 ++++
 modules/gallery/views/menu_link.html.php      |  9 ++++
 7 files changed, 74 insertions(+), 53 deletions(-)
 create mode 100644 modules/gallery/views/menu.html.php
 create mode 100644 modules/gallery/views/menu_ajax_link.html.php
 create mode 100644 modules/gallery/views/menu_dialog.html.php
 create mode 100644 modules/gallery/views/menu_link.html.php

(limited to 'modules')

diff --git a/modules/gallery/libraries/Admin_View.php b/modules/gallery/libraries/Admin_View.php
index 6eedec0d..4cbefbbd 100644
--- a/modules/gallery/libraries/Admin_View.php
+++ b/modules/gallery/libraries/Admin_View.php
@@ -47,8 +47,7 @@ class Admin_View_Core extends Gallery_View {
   public function admin_menu() {
     $menu = Menu::factory("root");
     module::event("admin_menu", $menu, $this);
-    $menu->compact();
-    return $menu;
+    return $menu->compact()->render();
   }
 
   /**
diff --git a/modules/gallery/libraries/Menu.php b/modules/gallery/libraries/Menu.php
index 47af8531..22261557 100644
--- a/modules/gallery/libraries/Menu.php
+++ b/modules/gallery/libraries/Menu.php
@@ -80,19 +80,10 @@ class Menu_Element {
  * Menu element that provides a link to a new page.
  */
 class Menu_Element_Link extends Menu_Element {
-  public function __toString() {
-    if (isset($this->css_id) && !empty($this->css_id)) {
-      $css_id = " id=\"$this->css_id\"";
-    } else {
-      $css_id = "";
-    }
-    if (isset($this->css_class) && !empty($this->css_class)) {
-      $css_class = " $this->css_class";
-    } else {
-      $css_class = "";
-    }
-    return "<li><a$css_id class=\"g-menu-link $css_class\" href=\"$this->url\" " .
-      "title=\"$this->label\">$this->label</a></li>";
+  public function render() {
+    $view = new View("menu_link.html");
+    $view->menu = $this;
+    return $view;
   }
 }
 
@@ -111,19 +102,10 @@ class Menu_Element_Ajax_Link extends Menu_Element {
     return $this;
   }
 
-  public function __toString() {
-    if (isset($this->css_id) && !empty($this->css_id)) {
-      $css_id = " id=\"$this->css_id\"";
-    } else {
-      $css_id = "";
-    }
-    if (isset($this->css_class) && !empty($this->css_class)) {
-      $css_class = " $this->css_class";
-    } else {
-      $css_class = "";
-    }
-    return "<li><a$css_id class=\"g-ajax-link $css_class\" href=\"$this->url\" " .
-      "title=\"$this->label\" ajax_handler=\"$this->ajax_handler\">$this->label</a></li>";
+  public function render() {
+    $view = new View("menu_ajax_link.html");
+    $view->menu = $this;
+    return $view;
   }
 }
 
@@ -131,19 +113,10 @@ class Menu_Element_Ajax_Link extends Menu_Element {
  * Menu element that provides a pop-up dialog
  */
 class Menu_Element_Dialog extends Menu_Element {
-  public function __toString() {
-    if (isset($this->css_id) && !empty($this->css_id)) {
-      $css_id = " id=\"$this->css_id\"";
-    } else {
-      $css_id = "";
-    }
-    if (isset($this->css_class) && !empty($this->css_class)) {
-      $css_class = " $this->css_class";
-    } else {
-      $css_class = "";
-    }
-    return "<li><a$css_id class=\"g-dialog-link $css_class\" href=\"$this->url\" " .
-           "title=\"$this->label\">$this->label</a></li>";
+  public function render() {
+    $view = new View("menu_dialog.html");
+    $view->menu = $this;
+    return $view;
   }
 }
 
@@ -242,11 +215,9 @@ class Menu_Core extends Menu_Element {
     return null;
   }
 
-  public function __toString() {
-    $html = $this->is_root ? "<ul class=\"$this->css_class\">" :
-      "<li title=\"$this->label\"><a href=\"#\">$this->label</a><ul>";
-    $html .= implode("\n", $this->elements);
-    $html .= $this->is_root ? "</ul>" : "</ul></li>";
-    return $html;
+  public function render() {
+    $view = new View("menu.html");
+    $view->menu = $this;
+    return $view;
   }
 }
diff --git a/modules/gallery/libraries/Theme_View.php b/modules/gallery/libraries/Theme_View.php
index 68ec325f..d4d8ee2a 100644
--- a/modules/gallery/libraries/Theme_View.php
+++ b/modules/gallery/libraries/Theme_View.php
@@ -81,19 +81,19 @@ class Theme_View_Core extends Gallery_View {
   public function site_menu() {
     $menu = Menu::factory("root");
     module::event("site_menu", $menu, $this);
-    return $menu->compact();
+    return $menu->compact()->render();
   }
 
   public function album_menu() {
     $menu = Menu::factory("root");
     module::event("album_menu", $menu, $this);
-    return $menu->compact();
+    return $menu->compact()->render();
   }
 
   public function tag_menu() {
     $menu = Menu::factory("root");
     module::event("tag_menu", $menu, $this);
-    return $menu->compact();
+    return $menu->compact()->render();
   }
 
   public function photo_menu() {
@@ -107,13 +107,13 @@ class Theme_View_Core extends Gallery_View {
     }
 
     module::event("photo_menu", $menu, $this);
-    return $menu->compact();
+    return $menu->compact()->render();
   }
 
   public function movie_menu() {
     $menu = Menu::factory("root");
     module::event("movie_menu", $menu, $this);
-    return $menu->compact();
+    return $menu->compact()->render();
   }
 
   public function context_menu($item, $thumbnail_css_selector) {
@@ -124,7 +124,7 @@ class Theme_View_Core extends Gallery_View {
       ->css_class("g-context-menu");
 
     module::event("context_menu", $menu, $this, $item, $thumbnail_css_selector);
-    return $menu->compact();
+    return $menu->compact()->render();
   }
 
   public function pager() {
diff --git a/modules/gallery/views/menu.html.php b/modules/gallery/views/menu.html.php
new file mode 100644
index 00000000..0731c7b6
--- /dev/null
+++ b/modules/gallery/views/menu.html.php
@@ -0,0 +1,23 @@
+<?php defined("SYSPATH") or die("No direct script access.") ?>
+<? if ($menu->is_root): ?>
+
+<ul class="<?= $menu->css_class ?>">
+  <? foreach ($this->elements as $element): ?>
+  <?= $element->render() ?>
+  <? endforeach ?>
+</ul>
+
+<? else: ?>
+
+<li title="<?= $menu->label->for_html_attr() ?>">
+  <a href="#">
+    <?= $menu->label->for_html() ?>
+  </a>
+  <ul>
+    <? foreach ($this->elements as $element): ?>
+    <?= $element->render() ?>
+    <? endforeach ?>
+  </ul>
+</li>
+
+<? endif ?>
diff --git a/modules/gallery/views/menu_ajax_link.html.php b/modules/gallery/views/menu_ajax_link.html.php
new file mode 100644
index 00000000..00a394bc
--- /dev/null
+++ b/modules/gallery/views/menu_ajax_link.html.php
@@ -0,0 +1,10 @@
+<?php defined("SYSPATH") or die("No direct script access.") ?>
+<li>
+  <a id="<?= $menu->css_id ?>"
+     class="g-ajax-link <?= $menu->css_class ?>"
+     href="<?= $menu->url ?>"
+     title="<?= $menu->label->for_html_attr() ?>"
+     ajax_handler="<?= $menu->ajax_handler ?>">
+    <?= $menu->label->for_html() ?>
+  </a>
+</li>
diff --git a/modules/gallery/views/menu_dialog.html.php b/modules/gallery/views/menu_dialog.html.php
new file mode 100644
index 00000000..99b1b013
--- /dev/null
+++ b/modules/gallery/views/menu_dialog.html.php
@@ -0,0 +1,9 @@
+<?php defined("SYSPATH") or die("No direct script access.") ?>
+<li>
+  <a id="<?= $menu->css_id ?>"
+     class="g-dialog-link <?= $menu->css_class ?>"
+     href="<?= $menu->url ?>"
+     title="<?= $menu->label->for_html_attr() ?>">
+    <?= $menu->label->for_html() ?>
+  </a>
+</li>
diff --git a/modules/gallery/views/menu_link.html.php b/modules/gallery/views/menu_link.html.php
new file mode 100644
index 00000000..8e4cdb95
--- /dev/null
+++ b/modules/gallery/views/menu_link.html.php
@@ -0,0 +1,9 @@
+<?php defined("SYSPATH") or die("No direct script access.") ?>
+<li>
+  <a id="<?= $menu->css_id ?>"
+     class="g-menu-link <?= $menu->css_class ?>"
+     href="<?= $menu->url ?>"
+     title="<?= $menu->label->for_html_attr() ?>">
+    <?= $menu->label->for_html() ?>
+  </a>
+</li>
-- 
cgit v1.2.3


From 3b66ea3c3a4889c3ab3a2da0626fa5b554c1a7c9 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 27 Oct 2009 13:57:36 -0700
Subject: Oops.  Change "$this" to "$menu" in our menu component views.

---
 modules/gallery/views/menu.html.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'modules')

diff --git a/modules/gallery/views/menu.html.php b/modules/gallery/views/menu.html.php
index 0731c7b6..ba16ef36 100644
--- a/modules/gallery/views/menu.html.php
+++ b/modules/gallery/views/menu.html.php
@@ -2,7 +2,7 @@
 <? if ($menu->is_root): ?>
 
 <ul class="<?= $menu->css_class ?>">
-  <? foreach ($this->elements as $element): ?>
+  <? foreach ($menu->elements as $element): ?>
   <?= $element->render() ?>
   <? endforeach ?>
 </ul>
@@ -14,7 +14,7 @@
     <?= $menu->label->for_html() ?>
   </a>
   <ul>
-    <? foreach ($this->elements as $element): ?>
+    <? foreach ($menu->elements as $element): ?>
     <?= $element->render() ?>
     <? endforeach ?>
   </ul>
-- 
cgit v1.2.3


From 13faf6035d726f8a77b7dcac7e8e8fa3b24bfaf2 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 27 Oct 2009 14:00:32 -0700
Subject: Remove Menu::compact() in favor of putting an if-then clause in
 menu.html.php.  This serves two purposes:

1) It's more efficient since we're doing less passes over the Menu tree
2) We're allowing themers to decide whether or not to show empty menus
---
 modules/gallery/libraries/Admin_View.php |  2 +-
 modules/gallery/libraries/Menu.php       | 13 -------------
 modules/gallery/libraries/Theme_View.php | 12 ++++++------
 modules/gallery/views/menu.html.php      |  2 ++
 4 files changed, 9 insertions(+), 20 deletions(-)

(limited to 'modules')

diff --git a/modules/gallery/libraries/Admin_View.php b/modules/gallery/libraries/Admin_View.php
index 4cbefbbd..cbb781a1 100644
--- a/modules/gallery/libraries/Admin_View.php
+++ b/modules/gallery/libraries/Admin_View.php
@@ -47,7 +47,7 @@ class Admin_View_Core extends Gallery_View {
   public function admin_menu() {
     $menu = Menu::factory("root");
     module::event("admin_menu", $menu, $this);
-    return $menu->compact()->render();
+    return $menu->render();
   }
 
   /**
diff --git a/modules/gallery/libraries/Menu.php b/modules/gallery/libraries/Menu.php
index 22261557..fba21e09 100644
--- a/modules/gallery/libraries/Menu.php
+++ b/modules/gallery/libraries/Menu.php
@@ -155,19 +155,6 @@ class Menu_Core extends Menu_Element {
     }
   }
 
-  public function compact() {
-    foreach ($this->elements as $target_id => $element) {
-      if ($element->type == "submenu") {
-        if (empty($element->elements)) {
-          $this->remove($target_id);
-        } else {
-          $element->compact();
-        }
-      }
-    }
-    return $this;
-  }
-
   public function __construct($type) {
     parent::__construct($type);
     $this->elements = array();
diff --git a/modules/gallery/libraries/Theme_View.php b/modules/gallery/libraries/Theme_View.php
index d4d8ee2a..9670313d 100644
--- a/modules/gallery/libraries/Theme_View.php
+++ b/modules/gallery/libraries/Theme_View.php
@@ -81,19 +81,19 @@ class Theme_View_Core extends Gallery_View {
   public function site_menu() {
     $menu = Menu::factory("root");
     module::event("site_menu", $menu, $this);
-    return $menu->compact()->render();
+    return $menu->render();
   }
 
   public function album_menu() {
     $menu = Menu::factory("root");
     module::event("album_menu", $menu, $this);
-    return $menu->compact()->render();
+    return $menu->render();
   }
 
   public function tag_menu() {
     $menu = Menu::factory("root");
     module::event("tag_menu", $menu, $this);
-    return $menu->compact()->render();
+    return $menu->render();
   }
 
   public function photo_menu() {
@@ -107,13 +107,13 @@ class Theme_View_Core extends Gallery_View {
     }
 
     module::event("photo_menu", $menu, $this);
-    return $menu->compact()->render();
+    return $menu->render();
   }
 
   public function movie_menu() {
     $menu = Menu::factory("root");
     module::event("movie_menu", $menu, $this);
-    return $menu->compact()->render();
+    return $menu->render();
   }
 
   public function context_menu($item, $thumbnail_css_selector) {
@@ -124,7 +124,7 @@ class Theme_View_Core extends Gallery_View {
       ->css_class("g-context-menu");
 
     module::event("context_menu", $menu, $this, $item, $thumbnail_css_selector);
-    return $menu->compact()->render();
+    return $menu->render();
   }
 
   public function pager() {
diff --git a/modules/gallery/views/menu.html.php b/modules/gallery/views/menu.html.php
index ba16ef36..b4b38c93 100644
--- a/modules/gallery/views/menu.html.php
+++ b/modules/gallery/views/menu.html.php
@@ -1,4 +1,5 @@
 <?php defined("SYSPATH") or die("No direct script access.") ?>
+<? if ($menu->elements): // Don't show the menu if it has no choices ?>
 <? if ($menu->is_root): ?>
 
 <ul class="<?= $menu->css_class ?>">
@@ -21,3 +22,4 @@
 </li>
 
 <? endif ?>
+<? endif ?>
-- 
cgit v1.2.3