Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A few suggestions for Solar_View_Helper_Pager #9

Open
solarphp opened this issue Nov 28, 2010 · 0 comments
Open

A few suggestions for Solar_View_Helper_Pager #9

solarphp opened this issue Nov 28, 2010 · 0 comments

Comments

@solarphp
Copy link
Collaborator

Hey, Solar_View_Helper_Pager is such a simple and sweet helper. I like it a lot and have a couple of improvements to suggest:

  1. Don't require both $datacount? and $datapages? to be set. We can guess one by the other.
  2. Allow $data to be a optional parameter. If not set, it'll get the data from the view. With this, in most cases we can call only $this->pager() in a template.
  3. Allow the query string to be changed. This way it can be translated (which is what I really want) or we can use different query strings.

I've done these changes and added some more checkings before using $data and $config, so you can review these ideas in a patch:

--- /trunk/Solar/View/Helper/Pager.php  (revision 3429)
+++ /trunk/Solar/View/Helper/Pager.php  (working copy)
@@ -41,6 +41,10 @@
      * : (string) The locale key for the "next" link text.  Default is
      *   'PAGER_PREV'.
      * 
+     * `query`
+     * : (string) The locale key for the "page" url query string.  Default is
+     *   'PAGER_QUERY'.     
+     * 
      * `prev_class`
      * : (string) The CSS class for the previous-page <a> tag. Default is
      *   'prev'.
@@ -56,6 +60,9 @@
      * : (string) An HREF to the pager stylesheet to load. Default is
      *   'Solar/styles/pager.css'.
      * 
+     * `view_var`
+     * : (string) The name of the view variable with pager data.
+     * 
      * @var array
      * 
      */
@@ -65,10 +72,12 @@
         'div_class'  => 'pager',
         'prev'       => 'PAGER_PREV',
         'next'       => 'PAGER_NEXT',
+        'query'      => 'PAGER_QUERY',
         'prev_class' => 'prev',
         'curr_class' => 'curr',
         'next_class' => 'next',
         'style_href' => 'Solar/styles/pager.css',
+        'view_var'   => 'layout_pager',
     );

     /**
@@ -85,25 +94,54 @@
      * @return string
      * 
      */
-    public function pager($data, $config = null)
+    public function pager($data = null, $config = null)
     {
-        // info for the pager
-        $count  = $data['count'];
-        $pages  = $data['pages'];
-        $paging = $data['paging'];
-        $page   = $data['page'];
+        if ($config) {
+            // output config
+            $config = array_merge($this->_config, (array) $config);
+        } else {
+            $config = $this->_config;
+        }

-        // output config
-        $config = array_merge($this->_config, (array) $config);
+        if (! $data) {
+            // get data from view
+            $var = $config['view_var'];
+            $data = $this->_view->$var;
+        }        

+        $page   = (int) $data['page'];
+        $paging = (int) $data['paging'];
+        
+        if (! $page || !$paging) {
+            // invalid page or paging
+            return;
+        }
+        
+        if (isset($data['count']) && isset($data['pages'])) {
+            $count = (int) $data['count'];
+            $pages = (int) $data['pages'];
+        } elseif (isset($data['count'])) {
+            // calculate number of pages
+            $count = (int) $data['count'];
+            $pages = ceil($count / $paging);
+        } elseif (isset($data['pages'])) {
+            // guess the number of items: that's why 'count' is preferable
+            $pages = (int) $data['pages'];            
+            $count = $pages * $paging;
+        } else {
+            return;
+        }
+        
         // do we really need paging?
-        if ($pages <= 1) {
-            // zero or one pages, nothing to do here
+        if ($count <= 1 || $pages <= 1) {
+            // zero or one items/pages: nothing to do here
             return;
         }

-        // add the pager stylesheet
-        $this->_view->head()->addStyle($config['style_href']);
+        if ($config['style_href']) {
+            // add the pager stylesheet
+            $this->_view->head()->addStyle($config['style_href']);
+        }

         // make sure we have ol or ul
         $list_type = strtolower($config['list_type']);
@@ -114,7 +152,8 @@
         // get the base href to work with, and use str_replace on it later.
         // this will be faster than calling $uri->get() multiple times.
         $uri = Solar::factory('Solar_Uri_Action');
-        $uri->query['page'] = '__PAGE__';
+        $query_string = $this->_view->getText($config['query']);
+        $uri->query[$query_string] = '__PAGE__';

Thanks!

Hey, here's another suggestion: don't show 'prev' and 'next' items at all when there's no previous/next pages. The result is less confusing and makes more sense imo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

0 participants