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

Add second set of data table action icons below the report title #22827

Open
wants to merge 38 commits into
base: 5.x-dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
be6ebb9
WIP
michalkleiner Nov 29, 2024
580977b
Move top actions below the title
michalkleiner Dec 5, 2024
ff9fc8c
Use random id for search action
michalkleiner Dec 5, 2024
309029c
Allow for data table actions specific placement
michalkleiner Dec 5, 2024
575be25
Ensure search is functional and correctly focusing used input
michalkleiner Dec 5, 2024
f6de4b7
Revert random id var name
michalkleiner Dec 5, 2024
ac2c26a
Remove unused styles
michalkleiner Dec 5, 2024
d51ff23
Use custom variable for header actions
michalkleiner Dec 5, 2024
f7bd1ba
Hide top controls on dashboard
michalkleiner Dec 6, 2024
8a7af46
Hide top controls when report doesn't have data
michalkleiner Dec 6, 2024
c53f07d
Keep on-hover visibility of footer actions on dashboard
michalkleiner Dec 8, 2024
8360dae
No longer needed forced visibility on top actions
michalkleiner Dec 8, 2024
3a79df3
Focus search field on search action click
michalkleiner Dec 8, 2024
88a20f0
Show bottom actions on hover when empty data table (as it was before)
michalkleiner Dec 9, 2024
bef5dd6
Adjust spacing of top controls to be in line with bottom controls
michalkleiner Dec 9, 2024
542cabf
Further adjust spacing of top controls to be in line with bottom cont…
michalkleiner Dec 9, 2024
8194ec2
Fix indentation
michalkleiner Dec 9, 2024
2a388d3
Remove filter reset as that was breaking pagination and sorting
michalkleiner Dec 9, 2024
5b4216f
Update UI test screenshots
michalkleiner Dec 9, 2024
55d1bda
Hide top actions when previewing dashboard widget
michalkleiner Dec 10, 2024
7d0bef0
Use bottom data table actions in UI tests
michalkleiner Dec 10, 2024
c1aba60
Build vue files
michalkleiner Dec 10, 2024
a68e3bc
Update UI tests
michalkleiner Dec 10, 2024
8e875f4
Ensure the flatten report setting is removed from both top and bottom…
michalkleiner Dec 12, 2024
6a7fcfc
Ensure empty search results table displays top controls vs no data at…
michalkleiner Dec 12, 2024
193daf6
Merge branch '5.x-dev' into dev-13900
sgiehl Dec 16, 2024
a48064a
Ensure search fields correctly calculate their width
michalkleiner Dec 17, 2024
20d2983
Merge branch '5.x-dev' into dev-13900
michalkleiner Dec 17, 2024
229dd3a
Update UI test screenshots
michalkleiner Dec 17, 2024
8be965b
Update UI test unrelated to the PR but the icons change
michalkleiner Dec 18, 2024
3f2b0a3
Remove hover to display table controls in UI tests as always visible now
michalkleiner Dec 19, 2024
f73ed92
Adjust for tests
michalkleiner Dec 19, 2024
bc58b67
Fix UI tests
michalkleiner Dec 19, 2024
1008eaa
Update UI test screenshots
michalkleiner Dec 20, 2024
6dc8d38
Move mouse out of view to prevent unwanted hover
michalkleiner Dec 20, 2024
da3de87
Revert 'not is Widget' addition
michalkleiner Dec 20, 2024
7585e39
Update UI test screenshots
michalkleiner Dec 20, 2024
855edcc
Merge branch '5.x-dev' into dev-13900
michalkleiner Dec 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
52 changes: 42 additions & 10 deletions plugins/CoreHome/javascripts/dataTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ $.extend(DataTable.prototype, UIControl.prototype, {
'include_aggregate_rows',
'totalRows',
'pivotBy',
'pivotByColumn'
'pivotByColumn',
'filter_trigger_id'
];

for (var key = 0; key < filters.length; key++) {
Expand Down Expand Up @@ -893,14 +894,36 @@ $.extend(DataTable.prototype, UIControl.prototype, {
$searchAction.addClass('searchActive forceActionVisible');
var width = getOptimalWidthForSearchField();
mneudert marked this conversation as resolved.
Show resolved Hide resolved
$searchAction.css('width', width + 'px');
$searchAction.find('.dataTableSearchInput').focus();

if (typeof self.param.filter_trigger_id != "undefined"
&& self.param.filter_trigger_id.length > 0) {
var triggerField = document.getElementById(self.param.filter_trigger_id);
if (triggerField) {
triggerField.focus();
}
} else {
$(event.target).siblings('input').focus();
}

$searchAction.find('.icon-search').on('click', searchForPattern);
$searchAction.off('click', showSearch);
}

function searchForPattern() {
var keyword = $searchInput.val();
function searchForPattern(event) {
var keyword = '';
if (event) {
var $input;
if (event.target.tagName.toLowerCase() === 'input') {
$input = $(event.target);
} else if (event.target.tagName.toLowerCase() === 'span') {
$input = $(event.target).siblings('input');
}

if ($input && $input.length) {
keyword = $input.val();
self.param.filter_trigger_id = $input.attr('id');
}
}

if (!keyword && !currentPattern) {
// we search only if a keyword is actually given, or if no keyword is given and a search was performed
Expand Down Expand Up @@ -932,15 +955,19 @@ $.extend(DataTable.prototype, UIControl.prototype, {

$searchInput.on("keyup", function (e) {
if (isEnterKey(e)) {
searchForPattern();
searchForPattern(e);
} else if (isEscapeKey(e)) {
$searchAction.find('.icon-close').click();
}
});

const $dataTable = $searchInput.parents('.dataTable').first();
if (currentPattern) {
$dataTable.addClass('hasSearchKeyword');
$searchInput.val(currentPattern);
$searchAction.click();
} else {
$dataTable.removeClass('hasSearchKeyword');
}

if (this.isEmpty && !currentPattern) {
Expand Down Expand Up @@ -1239,10 +1266,15 @@ $.extend(DataTable.prototype, UIControl.prototype, {
if ((typeof self.numberOfSubtables == 'undefined' || self.numberOfSubtables == 0)
&& (typeof self.param.flat == 'undefined' || self.param.flat != 1)
) {
// if there are no subtables, remove the flatten action
const dataTableActionsVueApp = $('[vue-entry="CoreHome.DataTableActions"]', domElem).data('vueAppInstance');
if (dataTableActionsVueApp) {
dataTableActionsVueApp.showFlattenTable_ = false;
// if there are no subtables, remove the flatten action from all data table actions
const dataTableActionsVueApps = $('[vue-entry="CoreHome.DataTableActions"]', domElem);
if (dataTableActionsVueApps.length) {
dataTableActionsVueApps.each(function() {
const appData = $(this).data('vueAppInstance');
if (appData) {
appData.showFlattenTable_ = false;
}
});
}
}

Expand Down Expand Up @@ -1487,7 +1519,7 @@ $.extend(DataTable.prototype, UIControl.prototype, {
self.param.idSubtable = idSubTable;
self.param.action = self.props.subtable_controller_action;

delete self.param.totalRows;
delete self.param.totalRows;
Comment on lines -1490 to +1524
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually fixes the alignment in IDE


var extraParams = {};
extraParams.comparisonIdSubtables = self.getComparisonIdSubtables($(this));
Expand Down
18 changes: 17 additions & 1 deletion plugins/CoreHome/stylesheets/dataTable/_dataTable.less
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,16 @@ td.cellSubDataTable .loadingPiwik {
overflow-x: scroll;
}
}
.dataTableHeaderControls {
margin-top: -10px;
margin-bottom: 5px;

.widgetpreview-preview .widget &,
#dashboardWidgetsArea .widget &,
.dataTable.isDataTableEmpty:not(.hasSearchKeyword) & {
display: none;
}
}

.theWidgetContent .card .card-content, table.dataTable {
div.dataTableScroller {
Expand All @@ -665,7 +675,7 @@ td.cellSubDataTable .loadingPiwik {

@media only screen and (min-width: 993px) {
#dashboardWidgetsArea .widget,
.theWidgetContent > div:not(#dashboard) {
.theWidgetContent .isDataTableEmpty {
&:hover {
.limitSelection,
.dataTableControls .dataTableAction {
Expand All @@ -681,6 +691,12 @@ td.cellSubDataTable .loadingPiwik {
}
}
}
.theWidgetContent .isDataTableEmpty.hasSearchKeyword {
.limitSelection,
.dataTableControls .dataTableAction {
visibility: visible;
}
}
}

@media only screen and (max-width: 700px) {
Expand Down
17 changes: 16 additions & 1 deletion plugins/CoreHome/templates/_dataTable.twig
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@
{% set showCardAsContentBlock = (properties.show_as_content_block and properties.show_title and not isWidget) %}
{% set showOnlyTitleWithoutCard = not showCardAsContentBlock and properties.title and properties.show_title %}

{#
using the show_footer to control the header actions as well since this is going to be refactored with the goal
of moving the table actions to the top of the report for all reports, at which point the config for the report
can be renamed to show_header and show_header_icons
#}
{% set showTableActionsInHeader = properties.show_footer and properties.show_footer_icons %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{% set showTableActionsInHeader = properties.show_footer and properties.show_footer_icons %}
{% set showTableActionsInHeader = properties.show_footer and properties.show_footer_icons and not isWidget %}

I have not fully checked what more side effects this would have (other than the mentioned single widgets in EvolutionGraph_spec), but this would remove the slight layout change in the Dashboard_spec caused by some other CSS:

&:first-child {
padding-top:0;
}


{% if showCardAsContentBlock %}
<div class="card">
<div class="card-content">
Expand All @@ -30,7 +37,7 @@

{% set summaryRowId = constant('Piwik\\DataTable::ID_SUMMARY_ROW') %}{# ID_SUMMARY_ROW #}
{% set isSubtable = javascriptVariablesToSet.idSubtable is defined and javascriptVariablesToSet.idSubtable != 0 %}
<div class="dataTable {{ visualizationCssClass }} {{ properties.datatable_css_class|default('') }} {% if isSubtable %}subDataTable{% endif %} {% if isComparing|default(false) %}isComparing{% endif %}"
<div class="dataTable {{ visualizationCssClass }} {{ properties.datatable_css_class|default('') }}{% if isSubtable %} subDataTable{% endif %}{% if isComparing|default(false) %} isComparing{% endif %}{% if isDataTableEmpty %} isDataTableEmpty{% endif %}"
data-table-type="{{ properties.datatable_js_type }}"
data-report="{{ properties.report_id }}"
data-report-metadata="{{ reportMetdadata|json_encode|e('html_attr') }}"
Expand All @@ -50,6 +57,14 @@
{% if error is defined %}
<div vue-entry="CoreHome.Alert" severity="danger">{{ error.message }}</div>
{% else %}
{% if showTableActionsInHeader %}
<div class="row dataTableHeaderControls">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Live_spec tests have some selectors that get tripped by this new element:

var report = await page.$('.dataTableVizVisitorLog .card.row:first-child');

This would now need to be something like this:

var report = await page.jQuery('.dataTableVizVisitorLog .card.row:eq(1)');

And there are probably more selectors in that spec that need to be updated accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't trip on this, as the Visits Log does not have a card on the top level, so the first row for the table controls is not within a card.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the selectors

<div class="col dataTableControls">
{% include "@CoreHome/_dataTableActions.twig" with { placement: 'top' } %}
</div>
</div>
{% endif %}

{% if properties.show_header_message is defined and properties.show_header_message is not empty %}
<div class='datatableHeaderMessage'>{{ properties.show_header_message | raw }}</div>
{% endif %}
Expand Down
1 change: 1 addition & 0 deletions plugins/CoreHome/templates/_dataTableActions.twig
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@
view-data-table="{{ clientSideParameters.viewDataTable|json_encode }}"
pivot-dimension-name="{{ properties.pivot_dimension_name|default(null)|json_encode }}"
selectable-periods="{{ properties.selectable_periods|default([])|json_encode }}"
placement="{{ placement|default('footer')|json_encode }}"
></div>
Loading
Loading