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.

Sign up for GitHub

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

Dashboard auto-saving #3653

Merged
merged 6 commits into from
Apr 17, 2019
Merged

Dashboard auto-saving #3653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
53b7151
Dashboard auto-saving
ranbena Mar 28, 2019
bbb0d6b
Changed indication to “Saving…”
ranbena Apr 10, 2019
eef107c
Added clarification comment
ranbena Apr 11, 2019
7ef0ab1
Removed unneeded batchUpdate code
ranbena Apr 11, 2019
2ebbb4d
Fixed tests
ranbena Apr 12, 2019
fb5f66b
Disabled “Done Editing” button when saving
ranbena Apr 14, 2019
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
4 changes: 4 additions & 0 deletions client/app/assets/less/redash/redash-newstyle.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -865,4 +865,8 @@ text.slicetext {

.markdown strong {
font-weight: bold;
}

.disabled-silent {
pointer-events: none;
}
73 changes: 5 additions & 68 deletions client/app/components/dashboards/gridstack/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ function gridstack($parse, dashboardGridOptions) {
scope: {
editing: '=',
batchUpdate: '=', // set by directive - for using in wrapper components
onLayoutChanged: '=',
isOneColumnMode: '=',
},
controller() {
Expand Down Expand Up @@ -121,67 +122,6 @@ function gridstack($parse, dashboardGridOptions) {
});
};

this.batchUpdateWidgets = (items) => {
kravets-levko marked this conversation as resolved.
Show resolved Hide resolved
// This method is used to update multiple widgets with a single
// reflow (for example, restore positions when dashboard editing cancelled).
// "dirty" part of code: updating grid and DOM nodes directly.
// layout reflow is triggered by `batchUpdate`/`commit` calls
this.update((grid) => {
_.each(grid.grid.nodes, (node) => {
const item = items[node.id];
if (item) {
if (_.isNumber(item.col)) {
node.x = parseFloat(item.col);
node.el.attr('data-gs-x', node.x);
node._dirty = true;
}

if (_.isNumber(item.row)) {
node.y = parseFloat(item.row);
node.el.attr('data-gs-y', node.y);
node._dirty = true;
}

if (_.isNumber(item.sizeX)) {
node.width = parseFloat(item.sizeX);
node.el.attr('data-gs-width', node.width);
node._dirty = true;
}

if (_.isNumber(item.sizeY)) {
node.height = parseFloat(item.sizeY);
node.el.attr('data-gs-height', node.height);
node._dirty = true;
}

if (_.isNumber(item.minSizeX)) {
node.minWidth = parseFloat(item.minSizeX);
node.el.attr('data-gs-min-width', node.minWidth);
node._dirty = true;
}

if (_.isNumber(item.maxSizeX)) {
node.maxWidth = parseFloat(item.maxSizeX);
node.el.attr('data-gs-max-width', node.maxWidth);
node._dirty = true;
}

if (_.isNumber(item.minSizeY)) {
node.minHeight = parseFloat(item.minSizeY);
node.el.attr('data-gs-min-height', node.minHeight);
node._dirty = true;
}

if (_.isNumber(item.maxSizeY)) {
node.maxHeight = parseFloat(item.maxSizeY);
node.el.attr('data-gs-max-height', node.maxHeight);
node._dirty = true;
}
}
});
});
};

this.removeWidget = ($element) => {
const grid = this.grid();
if (grid) {
Expand Down Expand Up @@ -235,9 +175,7 @@ function gridstack($parse, dashboardGridOptions) {
};
},
link: ($scope, $element, $attr, controller) => {
const batchUpdateAssignable = _.isFunction($parse($attr.batchUpdate).assign);
const isOneColumnModeAssignable = _.isFunction($parse($attr.batchUpdate).assign);

const isOneColumnModeAssignable = _.isFunction($parse($attr.onLayoutChanged).assign);
let enablePolling = true;

$element.addClass('grid-stack');
Expand Down Expand Up @@ -300,6 +238,9 @@ function gridstack($parse, dashboardGridOptions) {
$(node.el).trigger('gridstack.changed', node);
}
});
if ($scope.onLayoutChanged) {
$scope.onLayoutChanged();
}
changedNodes = {};
});

Expand All @@ -315,10 +256,6 @@ function gridstack($parse, dashboardGridOptions) {
controller.setEditing(!!value);
});

if (batchUpdateAssignable) {
$scope.batchUpdate = controller.batchUpdateWidgets;
}

$scope.$on('$destroy', () => {
enablePolling = false;
controller.$el = null;
Expand Down
18 changes: 8 additions & 10 deletions client/app/pages/dashboards/dashboard.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,15 @@ <h3>
</div>
<div class="col-xs-4 col-sm-5 col-lg-5 text-right dashboard__control p-r-0">
<span ng-if="!$ctrl.dashboard.is_archived && !public" class="hidden-print">
<div class="btn-group">
<button type="button" class="btn btn-primary btn-sm"
ng-disabled="$ctrl.isGridDisabled"
ng-click="$ctrl.editLayout(false, true)" ng-if="$ctrl.layoutEditing">
<i class="zmdi zmdi-check"></i> Apply Changes
</button>
<div ng-if="$ctrl.layoutEditing" ng-switch="$ctrl.isLayoutDirty || $ctrl.saveInProgress">
Copy link
Member

Choose a reason for hiding this comment

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

Not an issue, but perhaps it would be interesting to have a "Failed to save" state as well. E.g.: When internet connection fails as the state would keep Dirty you would see a "Saving" state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point - in case of failure we would mislead the user.
I'll see if I have a quick and easy solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PR #3715

<span class="save-status" data-saving ng-switch-when="true">Saving</span>
<span class="save-status" ng-switch-default>Saved</span>

<button type="button" class="btn btn-default btn-sm"
<button type="button" class="btn btn-primary btn-sm"
ng-disabled="$ctrl.isGridDisabled"
ng-click="$ctrl.editLayout(false, false)" ng-if="$ctrl.layoutEditing">
<i class="zmdi zmdi-close"></i> Cancel
ng-click="$ctrl.editLayout(false)"
ng-class="{'disabled-silent': $ctrl.isLayoutDirty || $ctrl.saveInProgress }">
<i class="fa fa-check"></i> Done Editing
</button>
</div>

Expand Down Expand Up @@ -91,7 +89,7 @@ <h3>
</div>

<div style="padding-bottom: 5px;" ng-if="$ctrl.dashboard.widgets.length > 0">
<div gridstack editing="$ctrl.layoutEditing && !$ctrl.saveInProgress" batch-update="$ctrl.updateGridItems"
<div gridstack editing="$ctrl.layoutEditing" on-layout-changed="$ctrl.onLayoutChanged"
is-one-column-mode="$ctrl.isGridDisabled" class="dashboard-wrapper"
ng-class="{'preview-mode': !$ctrl.layoutEditing, 'editing-mode': $ctrl.layoutEditing}">
<div class="dashboard-widget-wrapper"
Expand Down
68 changes: 30 additions & 38 deletions client/app/pages/dashboards/dashboard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,34 +48,37 @@ function DashboardCtrl(
) {
this.saveInProgress = false;

const saveDashboardLayout = (widgets) => {
const saveDashboardLayout = () => {
if (!this.dashboard.canEdit()) {
return;
}

// calc diff, bail if none
const changedWidgets = getWidgetsWithChangedPositions(this.dashboard.widgets);
if (!changedWidgets.length) {
Copy link
Member

Choose a reason for hiding this comment

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

Lodash isEmpty is safer, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case I know for sure it's gonna be an array.

this.isLayoutDirty = false;
$scope.$applyAsync();
return;
}

this.saveInProgress = true;
const showMessages = true;
return $q
.all(_.map(widgets, widget => widget.save()))
.all(_.map(changedWidgets, widget => widget.save()))
.then(() => {
if (showMessages) {
notification.success('Changes saved.');
}
// Update original widgets positions
_.each(widgets, (widget) => {
_.extend(widget.$originalPosition, widget.options.position);
});
this.isLayoutDirty = false;
})
.catch(() => {
if (showMessages) {
notification.error('Error saving changes.');
}
// in the off-chance that a widget got deleted mid-saving it's position, an error will occur
// currently left unhandled PR 3653#issuecomment-481699053
notification.error('Error saving changes.');
})
.finally(() => {
this.saveInProgress = false;
});
};

const saveDashboardLayoutDebounced = _.debounce(saveDashboardLayout, 2000);

this.layoutEditing = false;
this.isFullscreen = false;
this.refreshRate = null;
Expand All @@ -84,6 +87,7 @@ function DashboardCtrl(
this.showPermissionsControl = clientConfig.showPermissionsControl;
this.globalParameters = [];
this.isDashboardOwner = false;
this.isLayoutDirty = false;

this.refreshRates = clientConfig.dashboardRefreshIntervals.map(interval => ({
name: durationHumanize(interval),
Expand Down Expand Up @@ -242,28 +246,17 @@ function DashboardCtrl(
});
};

this.editLayout = (enableEditing, applyChanges) => {
if (!this.isGridDisabled) {
if (!enableEditing) {
if (applyChanges) {
const changedWidgets = getWidgetsWithChangedPositions(this.dashboard.widgets);
saveDashboardLayout(changedWidgets);
} else {
// Revert changes
const items = {};
_.each(this.dashboard.widgets, (widget) => {
_.extend(widget.options.position, widget.$originalPosition);
items[widget.id] = widget.options.position;
});
this.dashboard.widgets = Dashboard.prepareWidgetsForDashboard(this.dashboard.widgets);
if (this.updateGridItems) {
this.updateGridItems(items);
}
}
}

this.layoutEditing = enableEditing;
this.onLayoutChanged = () => {
// prevent unnecessary save when gridstack is loaded
if (!this.layoutEditing) {
return;
}
this.isLayoutDirty = true;
saveDashboardLayoutDebounced();
};

this.editLayout = (enableEditing) => {
this.layoutEditing = enableEditing;
};

this.loadTags = () => getTags('api/dashboards/tags').then(tags => _.map(tags, t => t.name));
Expand Down Expand Up @@ -405,12 +398,11 @@ function DashboardCtrl(
this.removeWidget = (widgetId) => {
this.dashboard.widgets = this.dashboard.widgets.filter(w => w.id !== undefined && w.id !== widgetId);
this.extractGlobalParameters();
$scope.$applyAsync();

if (!this.layoutEditing) {
// We need to wait a bit while `angular` updates widgets, and only then save new layout
$timeout(() => {
const changedWidgets = getWidgetsWithChangedPositions(this.dashboard.widgets);
saveDashboardLayout(changedWidgets);
}, 50);
$timeout(saveDashboardLayout, 50);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the diffing into the debounced save.

}
};

Expand Down
32 changes: 32 additions & 0 deletions client/app/pages/dashboards/dashboard.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,38 @@
}
}

.dashboard__control {
.save-status {
vertical-align: middle;
margin-right: 7px;
font-size: 12px;
text-align: left;
display: inline-block;

&[data-saving] {
opacity: 0.6;
width: 45px;

&:after {
content: '';
animation: saving 2s linear infinite;
}
}
}
}

@keyframes saving {
0%, 100% {
content: '.';
}
33% {
content: '..';
}
66% {
content: '...';
}
}


// Mobile fixes
@media (max-width: 767px) {
Expand Down
6 changes: 6 additions & 0 deletions client/app/services/widget.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ function WidgetFactory($http, $location, Query, Visualization, dashboardGridOpti
this.options.position.autoHeight = true;
}

this.updateOriginalPosition();
}

updateOriginalPosition() {
// Save original position (create a shallow copy)
this.$originalPosition = extend({}, this.options.position);
}
Expand Down Expand Up @@ -161,6 +165,8 @@ function WidgetFactory($http, $location, Query, Visualization, dashboardGridOpti
this[k] = v;
});

this.updateOriginalPosition();

return this;
});
}
Expand Down
Loading