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

[WIP]: Internalize vis editor URL state and implement rough UI-based undo/redo. #8019

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion src/core_plugins/kibana/public/visualize/editor/editor.html
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,11 @@
<div class="vis-editor-content">

<div class="collapsible-sidebar" ng-if="chrome.getVisible()">
<vis-editor-sidebar class="vis-editor-sidebar"></vis-editor-sidebar>
<vis-editor-sidebar
class="vis-editor-sidebar"
undo-state-history="undoStateHistory()"
redo-state-history="redoStateHistory()"
></vis-editor-sidebar>
</div>

<div class="vis-editor-canvas" ng-class="{ embedded: !chrome.getVisible() }">
Expand Down
55 changes: 53 additions & 2 deletions src/core_plugins/kibana/public/visualize/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,19 @@ uiModules
'kibana/notify',
'kibana/courier'
])
.controller('VisEditor', function ($scope, $route, timefilter, AppState, $location, kbnUrl, $timeout, courier, Private, Promise) {
.controller('VisEditor', function (
$location,
$rootScope,
$route,
$scope,
$timeout,
AppState,
courier,
kbnUrl,
Private,
Promise,
timefilter,
) {

const docTitle = Private(DocTitleProvider);
const brushEvent = Private(UtilsBrushEventProvider);
Expand All @@ -70,6 +82,40 @@ uiModules

const vis = savedVis.vis;
const editableVis = vis.createEditableVis();

// Store history of visualization state.
const pastStateHistory = [];
let futureStateHistory = [];

function resetFutureStateHistory() {
futureStateHistory = [];
}

$scope.undoStateHistory = () => {
// Move present state into future.
futureStateHistory.push(vis.getState());

// Set past state to present.
const lastState = pastStateHistory.pop();
vis.setState(lastState);
editableVis.setState(lastState);
$scope.fetch();
};

$scope.redoStateHistory = () => {
// Move present state into the past.
pastStateHistory.push(vis.getState());

// Set future state to present.
const nextState = futureStateHistory.pop();
vis.setState(nextState);
editableVis.setState(nextState);
$scope.fetch();
};

// We intend to keep editableVis and vis in sync with one another, so calling `requesting` on
// vis should call it on both. However, it's not clear what is the benefit of calling
// `editableVis.requesting()`.
vis.requesting = function () {
const requesting = editableVis.requesting;
requesting.call(vis);
Expand Down Expand Up @@ -146,7 +192,12 @@ uiModules
editableVis.listeners.brush = vis.listeners.brush = brushEvent;

// track state of editable vis vs. "actual" vis
$scope.stageEditableVis = transferVisState(editableVis, vis, true);
$scope.stageEditableVis = () => {
resetFutureStateHistory();
pastStateHistory.push(vis.getState());
transferVisState(editableVis, vis, true)();
};

$scope.resetEditableVis = transferVisState(vis, editableVis);
$scope.$watch(function () {
return editableVis.getEnabledState();
Expand Down
6 changes: 6 additions & 0 deletions src/core_plugins/kibana/public/visualize/editor/sidebar.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@
</li>
</ul>

<!-- Undo/redo -->
<div>
<span ng-if="undoStateHistory" ng-click="undoStateHistory()">Undo</span>
<span ng-if="redoStateHistory" ng-click="redoStateHistory()">Redo</span>
</div>

<!-- controls -->
<ul class="nav navbar-nav navbar-right">
<li
Expand Down
2 changes: 0 additions & 2 deletions src/core_plugins/kibana/public/visualize/editor/sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import sidebarTemplate from 'plugins/kibana/visualize/editor/sidebar.html';
uiModules
.get('app/visualize')
.directive('visEditorSidebar', function () {


return {
restrict: 'E',
template: sidebarTemplate,
Expand Down
30 changes: 21 additions & 9 deletions src/ui/public/share/directives/share_object_url.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import uiModules from 'ui/modules';
import shareObjectUrlTemplate from 'ui/share/views/share_object_url.html';


app.directive('shareObjectUrl', function (Private, Notifier) {
app.directive('shareObjectUrl', function (Private, Notifier, $rootScope, $httpParamSerializer) {
const urlShortener = Private(LibUrlShortenerProvider);

return {
Expand Down Expand Up @@ -69,15 +69,27 @@ app.directive('shareObjectUrl', function (Private, Notifier) {
});
};

$scope.getUrl = function () {
let url = $location.absUrl();
if ($scope.shareAsEmbed) {
url = url.replace('?', '?embed=true&');
}
return url;
};
// $scope.getUrl = function () {
// let url = $location.absUrl();
// if ($scope.shareAsEmbed) {
// url = url.replace('?', '?embed=true&');
// }
// return url;
// };

$rootScope.$on('state:queryParamChange', (event, param, value) => {
// When there's a change to the state, build a new shared URL using the new query param.
let search = $location.search();
search[param] = value;
const url = `${$location.absUrl().split('?')[0]}?${$httpParamSerializer(search)}`;
updateUrl(url);
});

$rootScope.$broadcast('state:triggerQueryParamChange');

$scope.$watch('getUrl()', updateUrl);
// TODO: What other use cases does this address beyond query param changes?
// We need to address them too.
// $scope.$watch('getUrl()', updateUrl);
}
};
});
33 changes: 32 additions & 1 deletion src/ui/public/state_management/state.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import _ from 'lodash';
import angular from 'angular';
import rison from 'rison-node';
import applyDiff from 'ui/utils/diff_object';
import qs from 'ui/utils/query_string';
Expand Down Expand Up @@ -42,6 +43,9 @@ export default function StateProvider(Private, $rootScope, $location) {

// Initialize the State with fetch
self.fetch();

// External actors in the system can request the current url params to be re-broadcast.
$rootScope.$on('state:triggerQueryParamChange', this.broadcastQueryParams.bind(this));
}

State.prototype._readFromURL = function () {
Expand Down Expand Up @@ -73,6 +77,22 @@ export default function StateProvider(Private, $rootScope, $location) {
}

_.defaults(stash, this._defaults);

// Internalize vis state on `this`. Once the vis state is initially read from the URL, it will
// never be read from it again.
// TODO: Allow properties such as vis to be dynamically specified via a public method,
// e.g. `internalizeParam()`.
if (this.toObject().vis) {
stash.vis = this.toObject().vis;
}

// TODO: Because we are removing the vis prop from the URL, it's also removed from the history.
// This means that navigating to a different part of the app and then hitting the browser back
// button will take you to an empty visualization. One possible solution is to store the current
// vis in the history with replaceState and pass it in as the state arg. Then, when the user
// navigates away and then back to this route, we need to listen for onpopstate and use the state
// to build the visualization.

// apply diff to state from stash, will change state in place via side effect
let diffResults = applyDiff(this, stash);

Expand Down Expand Up @@ -103,14 +123,25 @@ export default function StateProvider(Private, $rootScope, $location) {
this.emit('save_with_changes', diffResults.keys);
}

// Prevent vis state from being persisted to the URL. It's now represented internally on `this`.
delete state.vis;

// persist the state in the URL
let search = $location.search();
search[this._urlParam] = this.toRISON();
// RISON-encode state instead of `this`, because we only want to persist certain properties to
// the URL, e.g. *not* vis state.
search[this._urlParam] = rison.encode(JSON.parse(angular.toJson(state)));
if (replace) {
$location.search(search).replace();
} else {
$location.search(search);
}

this.broadcastQueryParams();
};

State.prototype.broadcastQueryParams = function () {
$rootScope.$broadcast('state:queryParamChange', this._urlParam, this.toRISON());
};

/**
Expand Down