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

Enhance table navigation bars to support systems with larger number of items #2534

Closed
YongBoLiu opened this issue Mar 18, 2019 · 3 comments
Closed
Labels
enhancement General tag for an enhancement resolved A fixed issue
Milestone

Comments

@YongBoLiu
Copy link
Contributor

YongBoLiu commented Mar 18, 2019

Is your feature request related to a problem? Please describe.
Performance issue.
In a large system(with huge data across tables), to get total_rows is expensive performance effect. So they only return the sizeof($result) that with limited rows.
And users don't care about the page_list when there is so many data rows.

Describe the solution you'd like
Add a parameter $page_count , the flag default is true.
If true, works as before - show the page list base on the real $total_rows.
If false, show current page number only. The real $total_rows not needed, just the sizeof($result) that with limited rows as $total_rows.

Describe alternatives you've considered

--- /home/yboliu/my_git/cacti_develop/lib/html.php	2019-03-18 15:50:59.559839654 +0800
+++ lib/html.php	2019-03-18 15:46:51.578219325 +0800
@@ -486,11 +486,12 @@
    @arg $total_rows - the total number of rows in the navigation system
    @arg $object - the object types that is being displayed
    @arg $page_var - the object types that is being displayed
-   @arg $return_to - paint the resulting page into this dom object */
-function html_nav_bar($base_url, $max_pages, $current_page, $rows_per_page, $total_rows, $colspan=30, $object = '', $page_var = 'page', $return_to = '') {
+   @arg $return_to - paint the resulting page into this dom object 
+   @arg $page_count - provide a page count */
+function html_nav_bar($base_url, $max_pages, $current_page, $rows_per_page, $total_rows, $colspan=30, $object = '', $page_var = 'page', $return_to = '', $page_count = true) {
 	if ($object == '') $object = __('Rows');
 
-	if ($total_rows > $rows_per_page) {
+	if ($total_rows > $rows_per_page && $page_count) {
 		if (substr_count($base_url, '?') == 0) {
 			$base_url = trim($base_url) . '?';
 		} else {
@@ -511,11 +512,38 @@
 			</div>
 		</div>\n";
 	} elseif ($total_rows > 0) {
-		$nav = "<div class='navBarNavigation'>
-			<div class='navBarNavigationNone'>
-				" . __('All %d %s', $total_rows, $object) . "
-			</div>
-		</div>\n";
+		if($page_count || ($total_rows < $rows_per_page && $current_page ==1) ){
+			$nav = "<div class='navBarNavigation'>
+				<div class='navBarNavigationNone'>
+					" . __('All %d %s', $total_rows, $object) . "
+				</div>
+			</div>\n";
+		}else{
+			if (substr_count($base_url, '?') == 0) {
+				$base_url = trim($base_url) . '?';
+			} else {
+				$base_url = trim($base_url) . '&';
+			}
+			$url_page_select = "<ul class='pagination'>"; //for the same height as write in get_page_list()
+			$url_page_select .= "<li>$current_page</a></li>";
+			$url_page_select .= '</ul>';
+			$nav = "<div class='navBarNavigation'>
+				<div class='navBarNavigationPrevious'>
+					" . (($current_page > 1) ? "<a href='#' onClick='goto$page_var(" . ($current_page-1) . ");return false;'><i class='fa fa-angle-double-left previous'></i>" . __('Previous'). "</a>":"") . "
+				</div>
+				<div class='navBarNavigationCenter'>
+					" . __('Current Page: %s', $url_page_select) . "
+				</div>
+				<div class='navBarNavigationNext'>
+					" . ($total_rows >= $rows_per_page ? "<a href='#' onClick='goto$page_var(" . ($current_page+1) . ");return false;'>" . __('Next'). "<i class='fa fa-angle-double-right next'></i></a>":"") . "
+				</div>
+			</div>\n";
+			if ($return_to != '') {//code as in get_page_list()
+				$nav .= "<script type='text/javascript'>function goto$page_var(pageNo) { if (typeof url_graph === 'function') { var url_add=url_graph('') } else { var url_add=''; }; $.get('" . $base_url . "header=false&" . $page_var . "='+pageNo+url_add).done(function(data) { $('#$return_to').html(data); applySkin(); }); }</script>";
+			} else {
+				$nav .= "<script type='text/javascript'>function goto${page_var}(pageNo) { if (typeof url_graph === 'function') { var url_add=url_graph('') } else { var url_add=''; }; document.location='$base_url$page_var='+pageNo+url_add }</script>";
+			}
+		}
 	} else {
 		$nav = "<div class='navBarNavigation'>
 			<div class='navBarNavigationNone'>

Additional context
Add any other context or screenshots about the feature request here.
nav1

@netniV
Copy link
Member

netniV commented Mar 18, 2019

Seems like it would be OK, though it can be misleading if the total number of rows does in fact change whilst enumerating through that data set (eg, log files). It would improve performance for the more static tables (the majority of them) such as Device lists, Template lists, etc.

Create a PR for it and we can review it properly.

@netniV netniV added the enhancement General tag for an enhancement label Mar 18, 2019
@TheWitness
Copy link
Member

I missed this one. Seems like a good idea. Let me test this out to ensure that nothing breaks.

TheWitness added a commit that referenced this issue Mar 17, 2020
Enhance the html_nav_bar() in the lib/html.php to support larger system
@TheWitness TheWitness added the resolved A fixed issue label Mar 17, 2020
@TheWitness TheWitness added this to the 1.2.11 milestone Mar 17, 2020
@TheWitness
Copy link
Member

Done.

@netniV netniV changed the title Enhance the html_nav_bar() in the lib/html.php to support larger system Enhance the html_nav_bar() function to support systems with larger number of items Apr 5, 2020
@netniV netniV changed the title Enhance the html_nav_bar() function to support systems with larger number of items Enhance table navigation bars to support systems with larger number of items Apr 5, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement General tag for an enhancement resolved A fixed issue
Projects
None yet
Development

No branches or pull requests

3 participants