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

Fix: Asset preservation #25359

Merged
merged 5 commits into from
Nov 8, 2018
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ export class AssetManager extends React.PureComponent {
cannot be determined at this time.
</p>
</EuiText>
<EuiFlexGrid responsive="false" columns={4}>
<EuiFlexGrid responsive={false} columns={4}>
{this.props.assets.map(this.renderAsset)}
</EuiFlexGrid>
</EuiModalBody>
Expand Down
5 changes: 1 addition & 4 deletions x-pack/plugins/canvas/public/components/error/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,5 @@ export const Error = ({ payload }) => {
};

Error.propTypes = {
payload: PropTypes.shape({
info: PropTypes.object.isRequired,
error: PropTypes.object.isRequired,
}).isRequired,
payload: PropTypes.object.isRequired,
};
1 change: 1 addition & 0 deletions x-pack/plugins/canvas/public/state/initial_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { getDefaultWorkpad } from './defaults';
export const getInitialState = path => {
const state = {
app: {}, // Kibana stuff in here
assets: {}, // assets end up here
transient: {
canUserWrite: true,
fullscreen: false,
Expand Down
31 changes: 27 additions & 4 deletions x-pack/plugins/canvas/public/state/middleware/history.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,17 @@ import { isEqual } from 'lodash';
import { routes } from '../../apps';
import { historyProvider } from '../../lib/history_provider';
import { routerProvider } from '../../lib/router_provider';
import { get as fetchWorkpad } from '../../lib/workpad_service';
import { restoreHistory, undoHistory, redoHistory } from '../actions/history';
import { initializeWorkpad } from '../actions/workpad';
import { setAssets } from '../actions/assets';
import { isAppReady } from '../selectors/app';
import { getWorkpad } from '../selectors/workpad';

function getHistoryState(state) {
// this is what gets written to browser history
return state.persistent;
}

export const historyMiddleware = ({ dispatch, getState }) => {
// iterate over routes, injecting redux to action handlers
Expand Down Expand Up @@ -52,7 +60,19 @@ export const historyMiddleware = ({ dispatch, getState }) => {

// only restore the history on popState events with state
// this only happens when using back/forward with popState objects
if (isBrowserNav) return dispatch(restoreHistory(historyState));
if (isBrowserNav) {
// TODO: oof, this sucks. we can't just shove assets into history state because
// firefox is limited to 640k (wat!). so, when we see that the workpad id is changing,
// we instead just restore the assets, which ensures the overall state is correct.
// there must be a better way to handle history though...
const currentWorkpadId = getWorkpad(getState()).id;
if (currentWorkpadId !== historyState.workpad.id) {
const newWorkpad = await fetchWorkpad(historyState.workpad.id);
dispatch(setAssets(newWorkpad.assets));
}

return dispatch(restoreHistory(historyState));
}

// execute route action on pushState and popState events
if (isUrlChange) return await router.parse(pathname);
Expand Down Expand Up @@ -99,14 +119,17 @@ export const historyMiddleware = ({ dispatch, getState }) => {
// if app switched from not ready to ready, replace current state
// this allows the back button to work correctly all the way to first page load
if (!isAppReady(oldState) && isAppReady(newState)) {
history.replace(newState.persistent);
history.replace(getHistoryState(newState));
return;
}

// if the persistent state changed, push it into the history
if (!isEqual(newState.persistent, oldState.persistent)) {
const oldHistoryState = getHistoryState(oldState);
const historyState = getHistoryState(newState);
if (!isEqual(historyState, oldHistoryState)) {
// if there are pending route changes, just replace current route (to avoid extra back/forth history entries)
const useReplaceState = handlerState.pendingCount !== 0;
useReplaceState ? history.replace(newState.persistent) : history.push(newState.persistent);
useReplaceState ? history.replace(historyState) : history.push(historyState);
}
};
};