From b5ed458b466e37b4d70ee65cdeebc0d38c7dc9be Mon Sep 17 00:00:00 2001
From: Benaka Moorthi <benaka.moorthi@gmail.com>
Date: Tue, 9 Jul 2013 02:19:41 -0400
Subject: [PATCH] Refs #4041, cleaned up ViewDataTable, related classes, and
related templates:
- Remove uniqIdTable, chartDivId and related code as important element IDs are determined in datatable_manager.js now.
- Clarify _dataTable twig template + related css.
- Also added visiblity to Piwik_DataTable_Renderer_Php::renderException.
---
core/DataTable/Renderer/Php.php | 2 +-
core/ViewDataTable.php | 57 +------------------
core/ViewDataTable/GenerateGraphHTML.php | 2 -
.../GenerateGraphHTML/ChartEvolution.php | 14 -----
core/ViewDataTable/HtmlTable.php | 12 ++++
core/ViewDataTable/HtmlTable/AllColumns.php | 5 ++
core/ViewDataTable/HtmlTable/Goals.php | 5 ++
plugins/CoreHome/stylesheets/datatable.css | 13 ++---
plugins/CoreHome/templates/_dataTable.twig | 25 ++++----
.../CoreHome/templates/_dataTableActions.twig | 2 +-
.../_dataTableActions_recursive.twig | 2 +-
.../_dataTableActions_subDataTable.twig | 2 +-
.../CoreHome/templates/_dataTableCloud.twig | 2 +-
.../CoreHome/templates/_dataTableGraph.twig | 2 +-
plugins/Live/templates/getVisitorLog.twig | 6 +-
15 files changed, 50 insertions(+), 101 deletions(-)
diff --git a/core/DataTable/Renderer/Php.php b/core/DataTable/Renderer/Php.php
index 9c671c1201..cd2e7659fb 100644
--- a/core/DataTable/Renderer/Php.php
+++ b/core/DataTable/Renderer/Php.php
@@ -88,7 +88,7 @@ class Piwik_DataTable_Renderer_Php extends Piwik_DataTable_Renderer
*
* @return string
*/
- function renderException()
+ public function renderException()
{
$this->renderHeader();
diff --git a/core/ViewDataTable.php b/core/ViewDataTable.php
index 087819db45..3a5ce83b74 100644
--- a/core/ViewDataTable.php
+++ b/core/ViewDataTable.php
@@ -164,13 +164,6 @@ abstract class Piwik_ViewDataTable
*/
protected $columnsToDisplay = array();
- /**
- * Variable that is used as the DIV ID in the rendered HTML
- *
- * @var string
- */
- protected $uniqIdTable = null;
-
/**
* Method to be implemented by the ViewDataTable_*.
* This method should create and initialize a $this->view object @see Piwik_View_Interface
@@ -285,6 +278,7 @@ abstract class Piwik_ViewDataTable
$this->controllerActionCalledWhenRequestSubTable = $controllerActionCalledWhenRequestSubTable;
$this->idSubtable = Piwik_Common::getRequestVar('idSubtable', false, 'int');
+ $this->viewProperties['report_id'] = $currentControllerName . '.' . $currentControllerAction;
$this->viewProperties['show_goals'] = false;
$this->viewProperties['show_ecommerce'] = false;
$this->viewProperties['show_search'] = Piwik_Common::getRequestVar('show_search', true);
@@ -306,7 +300,6 @@ abstract class Piwik_ViewDataTable
$this->viewProperties['show_footer_icons'] = ($this->idSubtable == false);
$this->viewProperties['show_related_reports'] = Piwik_Common::getRequestVar('show_related_reports', true);
$this->viewProperties['apiMethodToRequestDataTable'] = $this->apiMethodToRequestDataTable;
- $this->viewProperties['uniqueId'] = $this->getUniqueIdViewDataTable();
$this->viewProperties['exportLimit'] = Piwik_Config::getInstance()->General['API_datatable_default_limit'];
$this->viewProperties['highlight_summary_row'] = false;
@@ -625,54 +618,6 @@ abstract class Piwik_ViewDataTable
{
}
- /**
- * Returns a unique ID for this ViewDataTable.
- * This unique ID is used in the Javascript code:
- * Any ajax loaded data is loaded within a DIV that has id=$unique_id
- * The jquery code then replaces the existing html div id=$unique_id in the code with this data.
- *
- * @see datatable.js
- * @return string
- */
- protected function loadUniqueIdViewDataTable()
- {
- // if we request a subDataTable the $this->currentControllerAction DIV ID is already there in the page
- // we make the DIV ID really unique by appending the ID of the subtable requested
- if ($this->idSubtable != 0 // parent DIV has a idSubtable = 0 but the html DIV must have the name of the module.action
- && $this->idSubtable !== false // case there is no idSubtable
- ) {
- // see also datatable.js (the ID has to match with the html ID created to be replaced by the result of the ajax call)
- $uniqIdTable = 'subDataTable_' . $this->idSubtable;
- } else {
- // the $uniqIdTable variable is used as the DIV ID in the rendered HTML
- // we use the current Controller action name as it is supposed to be unique in the rendered page
- $uniqIdTable = $this->currentControllerName . $this->currentControllerAction;
- }
- return $uniqIdTable;
- }
-
- /**
- * Sets the $uniqIdTable variable that is used as the DIV ID in the rendered HTML
- * @param $uniqIdTable
- */
- public function setUniqueIdViewDataTable($uniqIdTable)
- {
- $this->viewProperties['uniqueId'] = $uniqIdTable;
- $this->uniqIdTable = $uniqIdTable;
- }
-
- /**
- * Returns current value of $uniqIdTable variable that is used as the DIV ID in the rendered HTML
- * @return null|string
- */
- public function getUniqueIdViewDataTable()
- {
- if ($this->uniqIdTable == null) {
- $this->uniqIdTable = $this->loadUniqueIdViewDataTable();
- }
- return $this->uniqIdTable;
- }
-
/**
* Returns array of properties, eg. "show_footer", "show_search", etc.
*
diff --git a/core/ViewDataTable/GenerateGraphHTML.php b/core/ViewDataTable/GenerateGraphHTML.php
index 39dd0bf167..8c17e37d8d 100644
--- a/core/ViewDataTable/GenerateGraphHTML.php
+++ b/core/ViewDataTable/GenerateGraphHTML.php
@@ -114,7 +114,6 @@ abstract class Piwik_ViewDataTable_GenerateGraphHTML extends Piwik_ViewDataTable
$originalViewDataTable = $original['viewDataTable'];
$result = $this->parametersToModify + $original;
- ;
$result['viewDataTable'] = $originalViewDataTable;
return $result;
@@ -153,7 +152,6 @@ abstract class Piwik_ViewDataTable_GenerateGraphHTML extends Piwik_ViewDataTable
$view->width = $this->width;
$view->height = $this->height;
- $view->chartDivId = $this->getUniqueIdViewDataTable() . "Chart";
$view->graphType = $this->graphType;
$view->data = $this->graphData;
diff --git a/core/ViewDataTable/GenerateGraphHTML/ChartEvolution.php b/core/ViewDataTable/GenerateGraphHTML/ChartEvolution.php
index 2a3f921405..4381b75855 100644
--- a/core/ViewDataTable/GenerateGraphHTML/ChartEvolution.php
+++ b/core/ViewDataTable/GenerateGraphHTML/ChartEvolution.php
@@ -72,20 +72,6 @@ class Piwik_ViewDataTable_GenerateGraphHTML_ChartEvolution extends Piwik_ViewDat
return $result;
}
- /**
- * We ensure that the graph for a given Goal has a different ID than the 'Goals Overview' graph
- * so that both can display on the dashboard at the same time
- * @return null|string
- */
- public function getUniqueIdViewDataTable()
- {
- $id = parent::getUniqueIdViewDataTable();
- if (!empty($this->parametersToModify['idGoal'])) {
- $id .= $this->parametersToModify['idGoal'];
- }
- return $id;
- }
-
/**
* Sets the columns that will be displayed on output evolution chart
* By default all columns are displayed ($columnsNames = array() will display all columns)
diff --git a/core/ViewDataTable/HtmlTable.php b/core/ViewDataTable/HtmlTable.php
index f1b2cca44c..97f02e3595 100644
--- a/core/ViewDataTable/HtmlTable.php
+++ b/core/ViewDataTable/HtmlTable.php
@@ -83,11 +83,17 @@ class Piwik_ViewDataTable_HtmlTable extends Piwik_ViewDataTable
Piwik::log("Failed to get data from API: " . $e->getMessage());
$this->isDataAvailable = false;
+ $this->loadingError = array('message' => $e->getMessage());
}
$this->postDataTableLoadedFromAPI();
$this->view = $this->buildView();
}
+
+ public function getDataTableType()
+ {
+ return 'dataTableNormal';
+ }
/**
* @return Piwik_View with all data set
@@ -95,6 +101,12 @@ class Piwik_ViewDataTable_HtmlTable extends Piwik_ViewDataTable
protected function buildView()
{
$view = new Piwik_View($this->dataTableTemplate);
+
+ $view->dataTableType = $this->getDataTableType();
+
+ if (!empty($this->loadingError)) {
+ $view->error = $this->loadingError;
+ }
if (!$this->isDataAvailable) {
$view->arrayDataTable = array();
diff --git a/core/ViewDataTable/HtmlTable/AllColumns.php b/core/ViewDataTable/HtmlTable/AllColumns.php
index dcd754de49..18e5b19e60 100644
--- a/core/ViewDataTable/HtmlTable/AllColumns.php
+++ b/core/ViewDataTable/HtmlTable/AllColumns.php
@@ -62,4 +62,9 @@ class Piwik_ViewDataTable_HtmlTable_AllColumns extends Piwik_ViewDataTable_HtmlT
return true;
}
+
+ public function getDataTableType()
+ {
+ return 'dataTableAllColumns';
+ }
}
diff --git a/core/ViewDataTable/HtmlTable/Goals.php b/core/ViewDataTable/HtmlTable/Goals.php
index 2ceb8e88c3..fab237e890 100644
--- a/core/ViewDataTable/HtmlTable/Goals.php
+++ b/core/ViewDataTable/HtmlTable/Goals.php
@@ -256,4 +256,9 @@ class Piwik_ViewDataTable_HtmlTable_Goals extends Piwik_ViewDataTable_HtmlTable
}
return true;
}
+
+ public function getDataTableType()
+ {
+ return 'dataTableGoals';
+ }
}
diff --git a/plugins/CoreHome/stylesheets/datatable.css b/plugins/CoreHome/stylesheets/datatable.css
index 11757a0ac0..c658b5022a 100644
--- a/plugins/CoreHome/stylesheets/datatable.css
+++ b/plugins/CoreHome/stylesheets/datatable.css
@@ -1,6 +1,5 @@
/*Overriding some dataTable css for better dashboard display*/
.widget .dataTableWrapper,
-.widget .dataTableAllColumnsWrapper,
.widget .dataTableGraphWrapper,
.widget .dataTableActionsWrapper,
.widget .dataTableGraphEvolutionWrapper {
@@ -16,20 +15,16 @@
}
/* container of each table */
-.dataTableWrapper {
+.dataTableNormal > .dataTableWrapper {
width: 450px;
/* not more than 450px to make sure 2 tables can fit horizontally on a 1024 screen */
}
-.dataTableAllColumnsWrapper {
+.dataTableAllColumns > .dataTableWrapper {
width: 535px;
}
-.subdataTableWrapper {
- width: 95%;
-}
-
-.subdataTableAllColumnsWrapper {
+.subDataTable > .dataTableWrapper {
width: 95%;
}
@@ -806,4 +801,4 @@ a.tableConfigurationIcon.highlighted {
table.dataTable span.cell-tooltip {
cursor: default;
-}
\ No newline at end of file
+}
diff --git a/plugins/CoreHome/templates/_dataTable.twig b/plugins/CoreHome/templates/_dataTable.twig
index 9548d3d9ef..8caf5b82da 100644
--- a/plugins/CoreHome/templates/_dataTable.twig
+++ b/plugins/CoreHome/templates/_dataTable.twig
@@ -1,23 +1,26 @@
-<div class="dataTable" data-table-type="dataTable" data-report="{{ properties.uniqueId }}" data-params="{{ javascriptVariablesToSet|json_encode }}">
+{% set isSubtable = javascriptVariablesToSet.idSubtable is defined and javascriptVariablesToSet.idSubtable != 0 %}
+<div class="dataTable {{ dataTableType }} {% if isSubtable %}subDataTable{% endif %}"
+ data-table-type="dataTable"
+ data-report="{{ properties.report_id }}"
+ data-params="{{ javascriptVariablesToSet|json_encode }}">
<div class="reportDocumentation">
- {% if reportDocumentation is not empty %}<p>{{ reportDocumentation|raw }}</p>{% endif %}
+ {% if reportDocumentation|default is not empty %}<p>{{ reportDocumentation|raw }}</p>{% endif %}
{% if properties.metadata.archived_date is defined %}<span class='helpDate'>{{ properties.metadata.archived_date }}</span>{% endif %}
</div>
- {% set class %}
- {% if javascriptVariablesToSet.idSubtable is defined and javascriptVariablesToSet.idSubtable != 0 %}sub {% endif %}{% if javascriptVariablesToSet.viewDataTable=='tableAllColumns' %}dataTableAllColumnsWrapper{% elseif javascriptVariablesToSet.viewDataTable=='tableGoals' %}dataTableAllColumnsWrapper {% else %}dataTableWrapper{% endif %}
- {% endset %}
- <div class="{{ class }}">
- {% if arrayDataTable.result is defined and arrayDataTable.result == 'error' %}
- {{ arrayDataTable.message }}
+ <div class="dataTableWrapper">
+ {% if error is defined %}
+ {{ error.message }}
{% else %}
{% if arrayDataTable|length == 0 %}
+ <div class="pk-emptyDataTable">
{% if showReportDataWasPurgedMessage is defined and showReportDataWasPurgedMessage %}
- <div class="pk-emptyDataTable">{{ 'CoreHome_DataForThisReportHasBeenPurged'|translate(deleteReportsOlderThan) }}</div>
+ {{ 'CoreHome_DataForThisReportHasBeenPurged'|translate(deleteReportsOlderThan) }}
{% else %}
- <div class="pk-emptyDataTable">{{ 'CoreHome_ThereIsNoDataForThisReport'|translate }}</div>
+ {{ 'CoreHome_ThereIsNoDataForThisReport'|translate }}
{% endif %}
+ </div>
{% else %}
- <table cellspacing="0" class="dataTable" id="{{ properties.uniqueId }}">
+ <table cellspacing="0" class="dataTable">
{% include "@CoreHome/_dataTableHead.twig" %}
<tbody>
diff --git a/plugins/CoreHome/templates/_dataTableActions.twig b/plugins/CoreHome/templates/_dataTableActions.twig
index 996d6c3c9e..2d167d56f4 100644
--- a/plugins/CoreHome/templates/_dataTableActions.twig
+++ b/plugins/CoreHome/templates/_dataTableActions.twig
@@ -1,4 +1,4 @@
-<div class="dataTable" data-table-type="actionDataTable" data-report="{{ properties.uniqueId }}" data-params="{{ javascriptVariablesToSet|json_encode }}">
+<div class="dataTable" data-table-type="actionDataTable" data-report="{{ properties.report_id }}" data-params="{{ javascriptVariablesToSet|json_encode }}">
<div class="reportDocumentation">
{% if reportDocumentation is not empty %}<p>{{ reportDocumentation|raw }}</p>{% endif %}
{% if properties.metadata.archived_date is defined %}<span class='helpDate'>{{ properties.metadata.archived_date }}</span>{% endif %}
diff --git a/plugins/CoreHome/templates/_dataTableActions_recursive.twig b/plugins/CoreHome/templates/_dataTableActions_recursive.twig
index 3426dd11ed..a5db06db7c 100644
--- a/plugins/CoreHome/templates/_dataTableActions_recursive.twig
+++ b/plugins/CoreHome/templates/_dataTableActions_recursive.twig
@@ -1,4 +1,4 @@
-<div class="dataTable" data-table-type="actionDataTable" data-report="{{ properties.uniqueId }}" data-params="{{ javascriptVariablesToSet|json_encode }}">
+<div class="dataTable" data-table-type="actionDataTable" data-report="{{ properties.report_id }}" data-params="{{ javascriptVariablesToSet|json_encode }}">
<div class="dataTableActionsWrapper">
{% if arrayDataTable.result is defined and arrayDataTable.result == 'error' %}
{{ arrayDataTable.message }}
diff --git a/plugins/CoreHome/templates/_dataTableActions_subDataTable.twig b/plugins/CoreHome/templates/_dataTableActions_subDataTable.twig
index 71d7fe7a4d..35a880f5ce 100644
--- a/plugins/CoreHome/templates/_dataTableActions_subDataTable.twig
+++ b/plugins/CoreHome/templates/_dataTableActions_subDataTable.twig
@@ -1,4 +1,4 @@
-<tr id="{{ properties.uniqueId }}"></tr>
+<tr id="subDataTable_{{ javascriptVariablesToSet.idSubtable }}"></tr>
{% if arrayDataTable.result is defined and arrayDataTable.result == 'error' %}
{{ arrayDataTable.message }}
{% else %}
diff --git a/plugins/CoreHome/templates/_dataTableCloud.twig b/plugins/CoreHome/templates/_dataTableCloud.twig
index 1e0a465a5a..4c70887307 100644
--- a/plugins/CoreHome/templates/_dataTableCloud.twig
+++ b/plugins/CoreHome/templates/_dataTableCloud.twig
@@ -1,4 +1,4 @@
-<div class="dataTable" data-report="{{ properties.uniqueId }}" data-params="{{ javascriptVariablesToSet|json_encode }}">
+<div class="dataTable" data-report="{{ properties.report_id }}" data-params="{{ javascriptVariablesToSet|json_encode }}">
{% if reportDocumentation is not empty and javascriptVariablesToSet.viewDataTable != 'tableGoals' %}
<div class="reportDocumentation"><p>{{ reportDocumentation|raw }}</p></div>
{% endif %}
diff --git a/plugins/CoreHome/templates/_dataTableGraph.twig b/plugins/CoreHome/templates/_dataTableGraph.twig
index 1e076d021b..2ebd657959 100644
--- a/plugins/CoreHome/templates/_dataTableGraph.twig
+++ b/plugins/CoreHome/templates/_dataTableGraph.twig
@@ -1,4 +1,4 @@
-<div class="dataTable" data-report="{{ properties.uniqueId }}" data-params="{{ javascriptVariablesToSet|json_encode }}">
+<div class="dataTable" data-report="{{ properties.report_id }}" data-params="{{ javascriptVariablesToSet|json_encode }}">
<div class="reportDocumentation">
{% if reportDocumentation is not empty %}<p>{{ reportDocumentation|raw }}</p>{% endif %}
diff --git a/plugins/Live/templates/getVisitorLog.twig b/plugins/Live/templates/getVisitorLog.twig
index bdb5574d6e..d7f439b735 100644
--- a/plugins/Live/templates/getVisitorLog.twig
+++ b/plugins/Live/templates/getVisitorLog.twig
@@ -1,4 +1,4 @@
-<div class="visitorLog dataTable" data-report="{{ properties.uniqueId }}" data-params="{{ javascriptVariablesToSet|json_encode }}">
+<div class="visitorLog dataTable" data-report="{{ properties.report_id }}" data-params="{{ javascriptVariablesToSet|json_encode }}">
{% if not isWidget %}
<h2>{% if javascriptVariablesToSet.filterEcommerce %}{{ 'Goals_EcommerceLog'|translate }}{% else %}{{ 'Live_VisitorLog'|translate }}{% endif %}</h2>
@@ -14,9 +14,9 @@
{{ arrayDataTable.message }}
{% else %}
{% if arrayDataTable|length == 0 %}
- <div class="pk-emptyDataTable" id="{{ properties.uniqueId }}">{{ 'CoreHome_ThereIsNoDataForThisReport'|translate }}</div>
+ <div class="pk-emptyDataTable">{{ 'CoreHome_ThereIsNoDataForThisReport'|translate }}</div>
{% else %}
- <table id="{{ properties.uniqueId }}" class="dataTable" cellspacing="0" width="100%" style="width:100%;">
+ <table class="dataTable" cellspacing="0" width="100%" style="width:100%;">
<thead>
<tr>
<th style="display:none"></th>
--
GitLab