From a2de44514c2cbc350893cc7ba10e2695f7dc82e3 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Fri, 11 Aug 2017 16:26:07 +0200 Subject: [PATCH] Smarter node details transition and polishing the edge cases. --- client/app/scripts/actions/app-actions.js | 19 ++++++++-------- client/app/scripts/components/node-details.js | 8 ++++--- client/app/scripts/constants/action-types.js | 1 - client/app/scripts/reducers/root.js | 22 ++++++------------- client/app/scripts/utils/time-utils.js | 6 +++++ client/app/scripts/utils/web-api-utils.js | 14 +++++------- 6 files changed, 33 insertions(+), 37 deletions(-) diff --git a/client/app/scripts/actions/app-actions.js b/client/app/scripts/actions/app-actions.js index c6476fbf3..960d71095 100644 --- a/client/app/scripts/actions/app-actions.js +++ b/client/app/scripts/actions/app-actions.js @@ -228,6 +228,7 @@ export function clickCloseDetails(nodeId) { type: ActionTypes.CLICK_CLOSE_DETAILS, nodeId }); + // Pull the most recent details for the next details panel that comes into focus. getNodeDetails(getState, dispatch); updateRoute(getState); }; @@ -539,20 +540,14 @@ export function receiveControlSuccess(nodeId) { }; } -export function receiveNodeDetails(details, timestamp = null) { +export function receiveNodeDetails(details, requestTimestamp) { return { type: ActionTypes.RECEIVE_NODE_DETAILS, - timestamp, + requestTimestamp, details }; } -export function nodeDetailsStartTransition() { - return { - type: ActionTypes.NODE_DETAILS_START_TRANSITION - }; -} - export function receiveNodesDelta(delta) { return (dispatch, getState) => { if (!isPausedSelector(getState())) { @@ -606,6 +601,9 @@ export function startTimeTravel() { if (isResourceViewModeSelector(getState())) { getResourceViewNodesSnapshot(getState(), dispatch); } + } else { + // Get most recent details before freezing the state. + getNodeDetails(getState, dispatch); } }; } @@ -739,10 +737,11 @@ export function receiveError(errorUrl) { }; } -export function receiveNotFound(nodeId) { +export function receiveNotFound(nodeId, requestTimestamp) { return { + type: ActionTypes.RECEIVE_NOT_FOUND, + requestTimestamp, nodeId, - type: ActionTypes.RECEIVE_NOT_FOUND }; } diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index b9c8dfaa7..151d627d0 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -8,6 +8,7 @@ import { clickCloseDetails, clickShowTopologyForNode } from '../actions/app-acti import { brightenColor, getNeutralColor, getNodeColorDark } from '../utils/color-utils'; import { isGenericTable, isPropertyList } from '../utils/node-details-utils'; import { resetDocumentTitle, setDocumentTitle } from '../utils/title-utils'; +import { timestampsEqual } from '../utils/time-utils'; import Overlay from './overlay'; import MatchedText from './matched-text'; @@ -139,6 +140,7 @@ class NodeDetails extends React.Component { Details will become available here when it communicates again.

+ ); } @@ -156,7 +158,7 @@ class NodeDetails extends React.Component { } renderDetails() { - const { details, nodeControlStatus, transitioning, nodeMatches = makeMap() } = this.props; + const { details, nodeControlStatus, nodeMatches = makeMap() } = this.props; const showControls = details.controls && details.controls.length > 0; const nodeColor = getNodeColorDark(details.rank, details.label, details.pseudo); const {error, pending} = nodeControlStatus ? nodeControlStatus.toJS() : {}; @@ -247,7 +249,7 @@ class NodeDetails extends React.Component { - + ); } @@ -287,8 +289,8 @@ class NodeDetails extends React.Component { function mapStateToProps(state, ownProps) { const currentTopologyId = state.get('currentTopologyId'); return { + transitioning: !timestampsEqual(state.get('pausedAt'), ownProps.timestamp), nodeMatches: state.getIn(['searchNodeMatches', currentTopologyId, ownProps.id]), - transitioning: state.get('pausedAt') && (!ownProps.timestamp || ownProps.timestamp.toISOString() !== state.get('pausedAt').toISOString()), nodes: state.get('nodes'), selectedNodeId: state.get('selectedNodeId'), }; diff --git a/client/app/scripts/constants/action-types.js b/client/app/scripts/constants/action-types.js index ed0ea006d..5238ad391 100644 --- a/client/app/scripts/constants/action-types.js +++ b/client/app/scripts/constants/action-types.js @@ -62,7 +62,6 @@ const ACTION_TYPES = [ 'SHOW_NETWORKS', 'SHUTDOWN', 'SORT_ORDER_CHANGED', - 'NODE_DETAILS_START_TRANSITION', 'START_TIME_TRAVEL', 'TIME_TRAVEL_START_TRANSITION', 'TOGGLE_CONTRAST_MODE', diff --git a/client/app/scripts/reducers/root.js b/client/app/scripts/reducers/root.js index afc5043dc..fccdae382 100644 --- a/client/app/scripts/reducers/root.js +++ b/client/app/scripts/reducers/root.js @@ -18,8 +18,9 @@ import { graphExceedsComplexityThreshSelector, isResourceViewModeSelector, } from '../selectors/topology'; +import { isPausedSelector } from '../selectors/time-travel'; import { activeTopologyZoomCacheKeyPathSelector } from '../selectors/zooming'; -import { nowInSecondsPrecision } from '../utils/time-utils'; +import { nowInSecondsPrecision, timestampsEqual } from '../utils/time-utils'; import { applyPinnedSearches } from '../utils/search-utils'; import { findTopologyById, @@ -543,8 +544,9 @@ export function rootReducer(state = initialState, action) { } case ActionTypes.RECEIVE_NODE_DETAILS: { - // Freeze node details data updates after the first load when paused. - if (state.getIn(['nodeDetails', action.details.id, 'details']) && state.get('pausedAt')) { + // Ignore the update if paused and the timestamp didn't change. + const setTimestamp = state.getIn(['nodeDetails', action.details.id, 'timestamp']); + if (isPausedSelector(state) && timestampsEqual(action.requestTimestamp, setTimestamp)) { return state; } @@ -552,26 +554,15 @@ export function rootReducer(state = initialState, action) { // disregard if node is not selected anymore if (state.hasIn(['nodeDetails', action.details.id])) { - console.log(action.timestamp && action.timestamp.toISOString()); state = state.updateIn(['nodeDetails', action.details.id], obj => ({ ...obj, notFound: false, - timestamp: action.timestamp, + timestamp: action.requestTimestamp, details: action.details, })); } return state; } - case ActionTypes.NODE_DETAILS_START_TRANSITION: { - const topNode = state.get('nodeDetails').last(); - if (topNode && topNode.id) { - state = state.updateIn(['nodeDetails', topNode.id], obj => ({ ...obj, - transitioning: true, - })); - } - return state; - } - case ActionTypes.SET_RECEIVED_NODES_DELTA: { // Turn on the table view if the graph is too complex, but skip // this block if the user has already loaded topologies once. @@ -638,6 +629,7 @@ export function rootReducer(state = initialState, action) { case ActionTypes.RECEIVE_NOT_FOUND: { if (state.hasIn(['nodeDetails', action.nodeId])) { state = state.updateIn(['nodeDetails', action.nodeId], obj => ({ ...obj, + timestamp: action.requestTimestamp, notFound: true, })); } diff --git a/client/app/scripts/utils/time-utils.js b/client/app/scripts/utils/time-utils.js index c93e42415..e0b576488 100644 --- a/client/app/scripts/utils/time-utils.js +++ b/client/app/scripts/utils/time-utils.js @@ -24,3 +24,9 @@ export function clampToNowInSecondsPrecision(timestamp) { export function scaleDuration(duration, scale) { return moment.duration(duration.asMilliseconds() * scale); } + +export function timestampsEqual(timestampA, timestampB) { + const stringifiedTimestampA = timestampA ? timestampA.toISOString() : ''; + const stringifiedTimestampB = timestampB ? timestampB.toISOString() : ''; + return stringifiedTimestampA === stringifiedTimestampB; +} diff --git a/client/app/scripts/utils/web-api-utils.js b/client/app/scripts/utils/web-api-utils.js index 0fd00eb12..f6086f643 100644 --- a/client/app/scripts/utils/web-api-utils.js +++ b/client/app/scripts/utils/web-api-utils.js @@ -3,11 +3,12 @@ import reqwest from 'reqwest'; import { defaults } from 'lodash'; import { Map as makeMap, List } from 'immutable'; -import { blurSearch, clearControlError, closeWebsocket, openWebsocket, receiveError, +import { + blurSearch, clearControlError, closeWebsocket, openWebsocket, receiveError, receiveApiDetails, receiveNodesDelta, receiveNodeDetails, receiveControlError, receiveControlNodeRemoved, receiveControlPipe, receiveControlPipeStatus, receiveControlSuccess, receiveTopologies, receiveNotFound, - receiveNodesForTopology, receiveNodes, nodeDetailsStartTransition + receiveNodesForTopology, receiveNodes, } from '../actions/app-actions'; import { getCurrentTopologyUrl } from '../utils/topology-utils'; @@ -283,6 +284,7 @@ export function getNodeDetails(getState, dispatch) { const nodeMap = state.get('nodeDetails'); const topologyUrlsById = state.get('topologyUrlsById'); const currentTopologyId = state.get('currentTopologyId'); + const requestTimestamp = state.get('pausedAt'); // get details for all opened nodes const obj = nodeMap.last(); @@ -294,29 +296,25 @@ export function getNodeDetails(getState, dispatch) { const topologyOptions = currentTopologyId === obj.topologyId ? activeTopologyOptionsSelector(state) : makeMap(); - const timestamp = state.get('pausedAt'); const query = buildUrlQuery(topologyOptions, state); if (query) { urlComponents = urlComponents.concat(['?', query]); } const url = urlComponents.join(''); - // if (isPausedSelector(state)) { - // dispatch(nodeDetailsStartTransition()); - // } doRequest({ url, success: (res) => { // make sure node is still selected if (nodeMap.has(res.node.id)) { - dispatch(receiveNodeDetails(res.node, timestamp)); + dispatch(receiveNodeDetails(res.node, requestTimestamp)); } }, error: (err) => { log(`Error in node details request: ${err.responseText}`); // dont treat missing node as error if (err.status === 404) { - dispatch(receiveNotFound(obj.id)); + dispatch(receiveNotFound(obj.id, requestTimestamp)); } else { dispatch(receiveError(topologyUrl)); }