Skip to content

Commit

Permalink
Remove graph CompressOnHide option.
Browse files Browse the repository at this point in the history
  • Loading branch information
jshaughn committed Jan 15, 2024
1 parent e73c46a commit 3561f1e
Show file tree
Hide file tree
Showing 17 changed files with 26 additions and 112 deletions.
1 change: 0 additions & 1 deletion frontend/cypress/integration/common/graph_display.ts
Expand Up @@ -131,7 +131,6 @@ Then('the display menu has default settings', () => {
cy.get(`input#trafficRate`).should('exist').should('not.be.checked');
cy.get(`input#boxByCluster`).should('exist').should('be.checked');
cy.get(`input#boxByNamespace`).should('exist').should('be.checked');
cy.get(`input#filterHide`).should('exist').should('be.checked');
cy.get(`input#filterIdleEdges`).should('exist').should('not.be.checked');
cy.get(`input#filterIdleNodes`).should('exist').should('not.be.checked');
cy.get(`input#filterOperationNodes`).should('exist').should('not.be.checked');
Expand Down
17 changes: 8 additions & 9 deletions frontend/cypress/integration/featureFiles/graph_display.feature
Expand Up @@ -8,7 +8,7 @@ Feature: Kiali Graph page - Display menu
Background:
Given user is at administrator perspective

# NOTE: Graph Find/Hide (compressOnHide) has its own test script
# NOTE: Graph Find/Hide has its own test script
# NOTE: Operation nodes has its own test script
# NOTE: Traffic animation, missing sidecars, virtual service, and idle edge options are nominally tested

Expand Down Expand Up @@ -221,11 +221,11 @@ Feature: Kiali Graph page - Display menu
Then user sees the "bookinfo" namespace
And only a single cluster box should be visible
Examples:
| type |
| APP |
| SERVICE |
| VERSIONED_APP|
| WORKLOAD |
| type |
| APP |
| SERVICE |
| VERSIONED_APP |
| WORKLOAD |

@skip
@multi-cluster
Expand All @@ -243,8 +243,8 @@ Feature: Kiali Graph page - Display menu
@multi-cluster
Scenario: Remote nodes should be restricted if user does not have access rights to a remote namespace
When user graphs "sleep" namespaces
And user "is" given access rights to a "sleep" namespace located in the "east" cluster
And user "is not" given access rights to a "sleep" namespace located in the "west" cluster
And user "is" given access rights to a "sleep" namespace located in the "east" cluster
And user "is not" given access rights to a "sleep" namespace located in the "west" cluster
And user is at the details page for the "app" "sleep/east" located in the "east" cluster
Then the nodes located in the "west" cluster should be restricted

Expand All @@ -257,4 +257,3 @@ Feature: Kiali Graph page - Display menu
And there is an Istio object in the "bookinfo" namespace for "east" cluster
And there is an Istio object in the "bookinfo" namespace for "west" cluster
Then the Istio objects for "bookinfo" should be grouped together in the panel

1 change: 0 additions & 1 deletion frontend/src/actions/ActionKeys.ts
Expand Up @@ -26,7 +26,6 @@ export enum ActionKeys {
// Toggle Actions
GRAPH_TOOLBAR_TOGGLE_BOX_BY_CLUSTER = 'GRAPH_TOOLBAR_TOGGLE_BOX_BY_CLUSTER',
GRAPH_TOOLBAR_TOGGLE_BOX_BY_NAMESPACE = 'GRAPH_TOOLBAR_TOGGLE_BOX_BY_NAMESPACE',
GRAPH_TOOLBAR_TOGGLE_COMPRESS_ON_HIDE = 'GRAPH_TOOLBAR_TOGGLE_COMPRESS_ON_HIDE',
GRAPH_TOOLBAR_TOGGLE_GRAPH_VIRTUAL_SERVICES = 'GRAPH_TOOLBAR_TOGGLE_GRAPH_VIRTUAL_SERVICES',
GRAPH_TOOLBAR_TOGGLE_GRAPH_MISSING_SIDECARS = 'GRAPH_TOOLBAR_TOGGLE_GRAPH_MISSING_SIDECARS',
GRAPH_TOOLBAR_TOGGLE_GRAPH_SECURITY = 'GRAPH_TOOLBAR_TOGGLE_GRAPH_SECURITY',
Expand Down
1 change: 0 additions & 1 deletion frontend/src/actions/GraphToolbarActions.ts
Expand Up @@ -15,7 +15,6 @@ export const GraphToolbarActions = {
// Toggle actions
toggleBoxByCluster: createAction(ActionKeys.GRAPH_TOOLBAR_TOGGLE_BOX_BY_CLUSTER),
toggleBoxByNamespace: createAction(ActionKeys.GRAPH_TOOLBAR_TOGGLE_BOX_BY_NAMESPACE),
toggleCompressOnHide: createAction(ActionKeys.GRAPH_TOOLBAR_TOGGLE_COMPRESS_ON_HIDE),
toggleLegend: createAction(ActionKeys.GRAPH_TOOLBAR_TOGGLE_LEGEND),
toggleGraphVirtualServices: createAction(ActionKeys.GRAPH_TOOLBAR_TOGGLE_GRAPH_VIRTUAL_SERVICES),
toggleGraphMissingSidecars: createAction(ActionKeys.GRAPH_TOOLBAR_TOGGLE_GRAPH_MISSING_SIDECARS),
Expand Down
1 change: 0 additions & 1 deletion frontend/src/app/History.ts
Expand Up @@ -42,7 +42,6 @@ export enum URLParam {
GRAPH_BADGE_VS = 'badgeVS',
GRAPH_BOX_CLUSTER = 'boxCluster',
GRAPH_BOX_NAMESPACE = 'boxNamespace',
GRAPH_COMPRESS_ON_HIDE = 'graphCompressOnHide',
GRAPH_EDGE_LABEL = 'edges',
GRAPH_EDGE_MODE = 'edgeMode',
GRAPH_FIND = 'graphFind',
Expand Down
2 changes: 0 additions & 2 deletions frontend/src/components/CytoscapeGraph/CytoscapeGraph.tsx
Expand Up @@ -48,7 +48,6 @@ import { PeerAuthentication } from 'types/IstioObjects';
import { ServiceDetailsInfo } from 'types/ServiceInfo';

type CytoscapeGraphProps = {
compressOnHide: boolean;
containerClassName?: string;
contextMenuEdgeComponent?: EdgeContextMenuComponentType;
contextMenuNodeComponent?: NodeContextMenuComponentType;
Expand Down Expand Up @@ -204,7 +203,6 @@ export class CytoscapeGraph extends React.Component<CytoscapeGraphProps, Cytosca
this.props.graphData.elements !== nextProps.graphData.elements ||
this.props.layout !== nextProps.layout ||
this.props.namespaceLayout !== nextProps.namespaceLayout ||
this.props.compressOnHide !== nextProps.compressOnHide ||
this.props.rankBy !== nextProps.rankBy ||
this.props.showOutOfMesh !== nextProps.showOutOfMesh ||
this.props.showRank !== nextProps.showRank ||
Expand Down
1 change: 0 additions & 1 deletion frontend/src/components/CytoscapeGraph/MiniGraphCard.tsx
Expand Up @@ -159,7 +159,6 @@ class MiniGraphCardComponent extends React.Component<MiniGraphCardProps, MiniGra
<CardBody>
<div style={{ height: '100%' }}>
<CytoscapeGraph
compressOnHide={true}
containerClassName={
this.props.graphContainerStyle ? this.props.graphContainerStyle : initGraphContainerStyle
}
Expand Down
Expand Up @@ -53,7 +53,6 @@ describe('CytoscapeGraph component test', () => {
dataSource.on('fetchSuccess', () => {
const wrapper = shallow(
<CytoscapeGraph
compressOnHide={true}
edgeLabels={myEdgeLabelMode}
edgeMode={EdgeMode.ALL}
graphData={{
Expand Down
3 changes: 1 addition & 2 deletions frontend/src/pages/Graph/GraphHelpFind.tsx
Expand Up @@ -52,8 +52,7 @@ export const GraphHelpFind: React.FC<GraphHelpFindProps> = (props: GraphHelpFind
const preface =
'You can use the Find and Hide fields to highlight or hide graph edges and nodes. Each field accepts ' +
'expressions using the language described below. Preset expressions are available via the dropdown. ' +
'Hide takes precedence when using Find and Hide together. Uncheck the "Compressed Hide" Display ' +
'option for hidden elements to retain their space.';
'Hide takes precedence when using Find and Hide together. ';

const edgeColumns: ThProps[] = [{ title: 'Expression' }, { title: 'Notes' }];

Expand Down
2 changes: 0 additions & 2 deletions frontend/src/pages/Graph/GraphPage.tsx
Expand Up @@ -114,7 +114,6 @@ type ReduxStateProps = {
activeTour?: TourInfo;
boxByCluster: boolean;
boxByNamespace: boolean;
compressOnHide: boolean;
duration: DurationInSeconds; // current duration (dropdown) setting
edgeLabels: EdgeLabelMode[];
edgeMode: EdgeMode;
Expand Down Expand Up @@ -882,7 +881,6 @@ const mapStateToProps = (state: KialiAppState): ReduxStateProps => ({
activeTour: state.tourState.activeTour,
boxByCluster: state.graph.toolbarState.boxByCluster,
boxByNamespace: state.graph.toolbarState.boxByNamespace,
compressOnHide: state.graph.toolbarState.compressOnHide,
duration: durationSelector(state),
edgeLabels: edgeLabelsSelector(state),
edgeMode: edgeModeSelector(state),
Expand Down
64 changes: 17 additions & 47 deletions frontend/src/pages/Graph/GraphToolbar/GraphFind.tsx
Expand Up @@ -35,7 +35,6 @@ import { isValid } from 'utils/Common';
import { serverConfig } from '../../../config';

type ReduxProps = {
compressOnHide: boolean;
edgeLabels: EdgeLabelMode[];
edgeMode: EdgeMode;
findValue: string;
Expand Down Expand Up @@ -209,8 +208,8 @@ export class GraphFindComponent extends React.Component<GraphFindProps, GraphFin
}
}

// We only update on a change to the find/hide/compress values, or a graph change. Although we use other props
// in processing (compressOnHide, layout, etc), a change to those settings will generate a graph change, so we
// We only update on a change to the find/hide values, or a graph change. Although we use other props
// in processing (layout, etc), a change to those settings will generate a graph change, so we
// wait for the graph change to do the update.
shouldComponentUpdate(nextProps: GraphFindProps, nextState: GraphFindState) {
const cyChanged = this.props.cy !== nextProps.cy;
Expand Down Expand Up @@ -288,15 +287,7 @@ export class GraphFindComponent extends React.Component<GraphFindProps, GraphFin
this.setHide(this.props.hideValue);
}

const compressOnHideChanged = this.props.compressOnHide !== prevProps.compressOnHide;
this.handleHide(
this.props.cy,
hideChanged,
graphChanged,
graphElementsChanged,
edgeModeChanged,
compressOnHideChanged
);
this.handleHide(this.props.cy, hideChanged, graphChanged, graphElementsChanged, edgeModeChanged);
}
}

Expand Down Expand Up @@ -539,8 +530,7 @@ export class GraphFindComponent extends React.Component<GraphFindProps, GraphFin
hideChanged: boolean,
graphChanged: boolean,
graphElementsChanged: boolean,
edgeModeChanged: boolean,
compressOnHideChanged: boolean
edgeModeChanged: boolean
) => {
const selector = this.parseValue(this.props.hideValue, false);
const checkRemovals = selector || this.props.edgeMode !== EdgeMode.ALL;
Expand Down Expand Up @@ -595,45 +585,26 @@ export class GraphFindComponent extends React.Component<GraphFindProps, GraphFin
}
}

if (this.props.compressOnHide) {
this.removedElements = cy.remove(hiddenElements);
// now subtract any boxes that no longer have children. this is iterative as boxes may be nested
let done = false;
while (!done) {
const emptyBoxes = cy.$('$node[isBox]').subtract(cy.$('$node[isBox] > :inside'));
if (emptyBoxes.length > 0) {
this.removedElements = this.removedElements.add(cy.remove(emptyBoxes));
} else {
done = true;
}
}
} else {
// set the remaining hide-hits hidden
this.hiddenElements = hiddenElements;
this.hiddenElements.kialiStyle({ visibility: 'hidden' });
// now subtract any visible boxes that don't have any visible children
let done = false;
while (!done) {
const emptyBoxes = cy.$('$node[isBox]:visible').subtract(cy.$('$node[isBox] > :visible'));
if (emptyBoxes.length > 0) {
emptyBoxes.kialiStyle({ visibility: 'hidden' });
this.hiddenElements = this.hiddenElements.add(emptyBoxes);
} else {
done = true;
}
// set the remaining hide-hits hidden
this.hiddenElements = hiddenElements;
this.hiddenElements.kialiStyle({ visibility: 'hidden' });
// now subtract any visible boxes that don't have any visible children
let done = false;
while (!done) {
const emptyBoxes = cy.$('$node[isBox]:visible').subtract(cy.$('$node[isBox] > :visible'));
if (emptyBoxes.length > 0) {
emptyBoxes.kialiStyle({ visibility: 'hidden' });
this.hiddenElements = this.hiddenElements.add(emptyBoxes);
} else {
done = true;
}
}
}

cy.endBatch();

const hasRemovedElements: boolean = !!this.removedElements && this.removedElements.length > 0;
if (
hideChanged ||
(compressOnHideChanged && checkRemovals) ||
(hasRemovedElements && graphElementsChanged) ||
edgeModeChanged
) {
if (hideChanged || (hasRemovedElements && graphElementsChanged) || edgeModeChanged) {
cy.emit('kiali-zoomignore', [true]);
CytoscapeGraphUtils.runLayout(cy, this.props.layout, this.props.namespaceLayout).then(() => {
// do nothing, defer to CytoscapeGraph.tsx 'onlayout' event handler
Expand Down Expand Up @@ -1121,7 +1092,6 @@ export class GraphFindComponent extends React.Component<GraphFindProps, GraphFin
}

const mapStateToProps = (state: KialiAppState) => ({
compressOnHide: state.graph.toolbarState.compressOnHide,
edgeLabels: edgeLabelsSelector(state),
edgeMode: edgeModeSelector(state),
findValue: findValueSelector(state),
Expand Down
32 changes: 0 additions & 32 deletions frontend/src/pages/Graph/GraphToolbar/GraphSettings.tsx
Expand Up @@ -40,7 +40,6 @@ import { serverConfig } from '../../../config';
type ReduxStateProps = {
boxByCluster: boolean;
boxByNamespace: boolean;
compressOnHide: boolean;
edgeLabels: EdgeLabelMode[];
rankBy: RankMode[];
showIdleEdges: boolean;
Expand All @@ -60,7 +59,6 @@ type ReduxDispatchProps = {
setRankBy: (rankBy: RankMode[]) => void;
toggleBoxByCluster(): void;
toggleBoxByNamespace(): void;
toggleCompressOnHide(): void;
toggleGraphMissingSidecars(): void;
toggleGraphSecurity(): void;
toggleGraphVirtualServices(): void;
Expand Down Expand Up @@ -143,13 +141,6 @@ class GraphSettingsComponent extends React.PureComponent<GraphSettingsProps, Gra
props.toggleBoxByNamespace
);

this.handleURLBool(
URLParam.GRAPH_COMPRESS_ON_HIDE,
INITIAL_GRAPH_STATE.toolbarState.compressOnHide,
props.compressOnHide,
props.toggleCompressOnHide
);

this.handleURLBool(
URLParam.GRAPH_IDLE_EDGES,
INITIAL_GRAPH_STATE.toolbarState.showIdleEdges,
Expand Down Expand Up @@ -259,13 +250,6 @@ class GraphSettingsComponent extends React.PureComponent<GraphSettingsProps, Gra
this.props.boxByNamespace
);

this.alignURLBool(
URLParam.GRAPH_COMPRESS_ON_HIDE,
INITIAL_GRAPH_STATE.toolbarState.compressOnHide,
prev.compressOnHide,
this.props.compressOnHide
);

this.alignURLBool(
URLParam.GRAPH_IDLE_EDGES,
INITIAL_GRAPH_STATE.toolbarState.showIdleEdges,
Expand Down Expand Up @@ -371,7 +355,6 @@ class GraphSettingsComponent extends React.PureComponent<GraphSettingsProps, Gra
const {
boxByCluster,
boxByNamespace,
compressOnHide,
edgeLabels,
showRank: rank,
rankBy: rankLabels,
Expand All @@ -390,7 +373,6 @@ class GraphSettingsComponent extends React.PureComponent<GraphSettingsProps, Gra
const {
toggleBoxByCluster,
toggleBoxByNamespace,
toggleCompressOnHide,
toggleGraphMissingSidecars,
toggleGraphSecurity,
toggleGraphVirtualServices,
Expand Down Expand Up @@ -563,18 +545,6 @@ class GraphSettingsComponent extends React.PureComponent<GraphSettingsProps, Gra
</div>
)
},
{
id: 'filterHide',
isChecked: compressOnHide,
labelText: 'Compressed Hide',
onChange: toggleCompressOnHide,
tooltip: (
<div style={{ textAlign: 'left' }}>
Compress the graph after graph-hide removes matching elements. Otherwise the graph maintains the space
consumed by the hidden elements.
</div>
)
},
{
id: 'filterIdleEdges',
isChecked: showIdleEdges,
Expand Down Expand Up @@ -1029,7 +999,6 @@ class GraphSettingsComponent extends React.PureComponent<GraphSettingsProps, Gra
const mapStateToProps = (state: KialiAppState): ReduxStateProps => ({
boxByCluster: state.graph.toolbarState.boxByCluster,
boxByNamespace: state.graph.toolbarState.boxByNamespace,
compressOnHide: state.graph.toolbarState.compressOnHide,
edgeLabels: edgeLabelsSelector(state),
showIdleEdges: state.graph.toolbarState.showIdleEdges,
showIdleNodes: state.graph.toolbarState.showIdleNodes,
Expand All @@ -1051,7 +1020,6 @@ const mapDispatchToProps = (dispatch: KialiDispatch): ReduxDispatchProps => {
setRankBy: bindActionCreators(GraphToolbarActions.setRankBy, dispatch),
toggleBoxByCluster: bindActionCreators(GraphToolbarActions.toggleBoxByCluster, dispatch),
toggleBoxByNamespace: bindActionCreators(GraphToolbarActions.toggleBoxByNamespace, dispatch),
toggleCompressOnHide: bindActionCreators(GraphToolbarActions.toggleCompressOnHide, dispatch),
toggleGraphMissingSidecars: bindActionCreators(GraphToolbarActions.toggleGraphMissingSidecars, dispatch),
toggleGraphSecurity: bindActionCreators(GraphToolbarActions.toggleGraphSecurity, dispatch),
toggleGraphVirtualServices: bindActionCreators(GraphToolbarActions.toggleGraphVirtualServices, dispatch),
Expand Down
Expand Up @@ -28,7 +28,6 @@ describe('Parse find value test', () => {
toggleGraphSecurity={testHandler}
toggleIdleNodes={testHandler}
toggleRank={testHandler}
compressOnHide={false}
layout={{ name: '' }}
namespaceLayout={{ name: '' }}
updateTime={0}
Expand Down
2 changes: 0 additions & 2 deletions frontend/src/pages/GraphPF/GraphPagePF.tsx
Expand Up @@ -107,7 +107,6 @@ type ReduxStateProps = {
activeTour?: TourInfo;
boxByCluster: boolean;
boxByNamespace: boolean;
compressOnHide: boolean;
duration: DurationInSeconds; // current duration (dropdown) setting
edgeLabels: EdgeLabelMode[];
edgeMode: EdgeMode;
Expand Down Expand Up @@ -729,7 +728,6 @@ const mapStateToProps = (state: KialiAppState): ReduxStateProps => ({
activeTour: state.tourState.activeTour,
boxByCluster: state.graph.toolbarState.boxByCluster,
boxByNamespace: state.graph.toolbarState.boxByNamespace,
compressOnHide: state.graph.toolbarState.compressOnHide,
duration: durationSelector(state),
edgeLabels: edgeLabelsSelector(state),
edgeMode: edgeModeSelector(state),
Expand Down
7 changes: 0 additions & 7 deletions frontend/src/reducers/GraphDataState.ts
Expand Up @@ -20,7 +20,6 @@ export const INITIAL_GRAPH_STATE: GraphState = {
toolbarState: {
boxByCluster: true,
boxByNamespace: true,
compressOnHide: true,
edgeLabels: [],
findValue: '',
graphType: GraphType.VERSIONED_APP,
Expand Down Expand Up @@ -148,12 +147,6 @@ export const GraphDataStateReducer = (state: GraphState = INITIAL_GRAPH_STATE, a
boxByNamespace: !state.toolbarState.boxByNamespace
})
});
case getType(GraphToolbarActions.toggleCompressOnHide):
return updateState(state, {
toolbarState: updateState(state.toolbarState, {
compressOnHide: !state.toolbarState.compressOnHide
})
});
case getType(GraphToolbarActions.toggleFindHelp):
return updateState(state, {
toolbarState: updateState(state.toolbarState, {
Expand Down
1 change: 0 additions & 1 deletion frontend/src/reducers/__tests__/GraphDataState.test.ts
Expand Up @@ -18,7 +18,6 @@ describe('GraphDataState', () => {
toolbarState: {
boxByCluster: true,
boxByNamespace: true,
compressOnHide: true,
edgeLabels: [],
findValue: '',
graphType: GraphType.VERSIONED_APP,
Expand Down

0 comments on commit 3561f1e

Please sign in to comment.