Skip to content

Commit

Permalink
Minimap props refactor
Browse files Browse the repository at this point in the history
Signed-off-by: Jitendra Gundaniya <[email protected]>
  • Loading branch information
jitu5 committed Jul 17, 2024
1 parent f8f0780 commit 46ee43e
Show file tree
Hide file tree
Showing 13 changed files with 46 additions and 51 deletions.
6 changes: 3 additions & 3 deletions src/actions/actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,12 @@ describe('actions', () => {
});

it('should create an action to toggle whether to show minimap', () => {
const visible = false;
const display = false;
const expectedAction = {
type: TOGGLE_MINIMAP,
visible,
display,
};
expect(toggleMiniMap(visible)).toEqual(expectedAction);
expect(toggleMiniMap(display)).toEqual(expectedAction);
});

it('should create an action to toggle whether to show the export modal', () => {
Expand Down
6 changes: 3 additions & 3 deletions src/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,12 @@ export const TOGGLE_MINIMAP = 'TOGGLE_MINIMAP';

/**
* Toggle mini map
* @param {String} visible Visibility status
* @param {String} display display status
*/
export function toggleMiniMap(visible) {
export function toggleMiniMap(display) {
return {
type: TOGGLE_MINIMAP,
visible,
display,
};
}

Expand Down
1 change: 0 additions & 1 deletion src/components/global-toolbar/global-toolbar.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ describe('GlobalToolbar', () => {
graph: true,
labelBtn: true,
layerBtn: true,
miniMap: true,
miniMapBtn: true,
modularPipelineFocusMode: null,
metadataModal: false,
Expand Down
24 changes: 11 additions & 13 deletions src/components/minimap-toolbar/minimap-toolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,17 @@ export const MiniMapToolbar = ({
return (
<>
<ul className="pipeline-minimap-toolbar kedro">
{displayMiniMap && (
<IconButton
active={visible.miniMap}
ariaLabel={`Turn minimap ${visible.miniMap ? 'off' : 'on'}`}
className={'pipeline-minimap-button pipeline-minimap-button--map'}
dataTest={`btnToggleMinimap`}
icon={MapIcon}
id="minimap-toggle-icon"
labelText={`${visible.miniMap ? 'Hide' : 'Show'} minimap`}
onClick={() => onToggleMiniMap(!visible.miniMap)}
visible={visible.miniMapBtn}
/>
)}
<IconButton
active={displayMiniMap}
ariaLabel={`Turn minimap ${displayMiniMap ? 'off' : 'on'}`}
className={'pipeline-minimap-button pipeline-minimap-button--map'}
dataTest={`btnToggleMinimap`}
icon={MapIcon}
id="minimap-toggle-icon"
labelText={`${displayMiniMap ? 'Hide' : 'Show'} minimap`}
onClick={() => onToggleMiniMap(!displayMiniMap)}
visible={visible.miniMapBtn}
/>
<IconButton
ariaLabel={'Zoom in'}
className={'pipeline-minimap-button pipeline-minimap-button--zoom-in'}
Expand Down
11 changes: 0 additions & 11 deletions src/components/minimap-toolbar/minimap-toolbar.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,11 @@ describe('MiniMapToolbar', () => {
}
);

it('does not display the toggle minimap button if displayMinimap is false', () => {
const props = {
chartZoom: { scale: 1, minScale: 0.5, maxScale: 1.5 },
displayMiniMap: false,
visible: { miniMap: false },
};
const wrapper = setup.mount(<MiniMapToolbar {...props} />);
expect(wrapper.find('.pipeline-minimap-button--map').length).toBe(0);
});

it('maps state to props', () => {
const expectedResult = {
displayMiniMap: true,
chartZoom: expect.any(Object),
visible: expect.objectContaining({
miniMap: expect.any(Boolean),
miniMapBtn: expect.any(Boolean),
}),
};
Expand Down
14 changes: 7 additions & 7 deletions src/components/minimap/minimap.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,14 @@ export class MiniMap extends Component {
* Updates drawing and zoom if props have changed
*/
update(prevProps = {}) {
const { visible, chartZoom } = this.props;
const { displayMiniMap, chartZoom } = this.props;

if (visible) {
if (displayMiniMap) {
const changed = (...names) => this.changed(names, prevProps, this.props);

if (
changed(
'visible',
'displayMiniMap',
'nodes',
'clickedNodes',
'linkedNodes',
Expand All @@ -113,11 +113,11 @@ export class MiniMap extends Component {
drawNodes.call(this);
}

if (changed('visible', 'chartZoom') && chartZoom.applied) {
if (changed('displayMiniMap', 'chartZoom') && chartZoom.applied) {
drawViewport.call(this);
}

if (changed('visible', 'nodes', 'textLabels', 'chartSize')) {
if (changed('displayMiniMap', 'nodes', 'textLabels', 'chartSize')) {
this.resetView();
}
}
Expand Down Expand Up @@ -329,7 +329,7 @@ export class MiniMap extends Component {
return (
<div
className="pipeline-minimap-container"
style={this.props.visible ? transformStyle : {}}
style={this.props.displayMiniMap ? transformStyle : {}}
>
<div
className="pipeline-minimap kedro"
Expand Down Expand Up @@ -401,7 +401,7 @@ const emptyNodes = [];
const emptyGraphSize = {};

export const mapStateToProps = (state, ownProps) => ({
visible: state.visible.miniMap,
displayMiniMap: state.display.miniMap,
mapSize: getMapSize(state),
clickedNode: state.node.clicked,
chartSize: getChartSize(state),
Expand Down
4 changes: 2 additions & 2 deletions src/components/minimap/minimap.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('MiniMap', () => {
});

it('does not update nodes when not visible', () => {
const wrapper = setup.mount(<MiniMap visible={false} />);
const wrapper = setup.mount(<MiniMap displayMiniMap={false} />);
const nodes = wrapper.render().find('.pipeline-minimap-node');
expect(nodes.length).toEqual(0);
});
Expand Down Expand Up @@ -144,7 +144,7 @@ describe('MiniMap', () => {

it('maps state to props', () => {
const expectedResult = {
visible: expect.any(Boolean),
displayMiniMap: expect.any(Boolean),
mapSize: expect.any(Object),
clickedNode: null,
chartSize: expect.any(Object),
Expand Down
15 changes: 15 additions & 0 deletions src/reducers/display.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { TOGGLE_MINIMAP } from '../actions';

function displayReducer(displayState = {}, action) {
switch (action.type) {
case TOGGLE_MINIMAP: {
return Object.assign({}, displayState, {
miniMap: action.display,
});
}
default:
return displayState;
}
}

export default displayReducer;
3 changes: 2 additions & 1 deletion src/reducers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import pipeline from './pipeline';
import tag from './tags';
import modularPipeline from './modular-pipelines';
import visible from './visible';
import display from './display';
import {
RESET_DATA,
TOGGLE_SHOW_FEATURE_HINTS,
Expand Down Expand Up @@ -66,8 +67,8 @@ const combinedReducer = combineReducers({
modularPipeline,
visible,
runsMetadata,
display,
// These props don't have any actions associated with them
display: createReducer(null),
dataSource: createReducer(null),
edge: createReducer({}),
// These props have very simple non-nested actions
Expand Down
4 changes: 2 additions & 2 deletions src/reducers/reducers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,9 @@ describe('Reducer', () => {
it('should toggle whether the minimap is open', () => {
const newState = reducer(mockState.spaceflights, {
type: TOGGLE_MINIMAP,
visible: false,
display: false,
});
expect(newState.visible.miniMap).toEqual(false);
expect(newState.display.miniMap).toEqual(false);
});
});

Expand Down
7 changes: 0 additions & 7 deletions src/reducers/visible.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {
TOGGLE_EXPORT_MODAL,
TOGGLE_GRAPH,
TOGGLE_METADATA_MODAL,
TOGGLE_MINIMAP,
TOGGLE_MODULAR_PIPELINE_FOCUS_MODE,
TOGGLE_SETTINGS_MODAL,
TOGGLE_SHAREABLE_URL_MODAL,
Expand Down Expand Up @@ -48,12 +47,6 @@ function visibleReducer(visibleState = {}, action) {
});
}

case TOGGLE_MINIMAP: {
return Object.assign({}, visibleState, {
miniMap: action.visible,
});
}

case TOGGLE_CODE: {
return Object.assign({}, visibleState, {
code: action.visible,
Expand Down
1 change: 0 additions & 1 deletion src/store/initial-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export const createInitialState = () => ({
labelBtn: true,
layerBtn: true,
metadataModal: false,
miniMap: true,
miniMapBtn: true,
modularPipelineFocusMode: null,
pipelineBtn: true,
Expand Down
1 change: 1 addition & 0 deletions src/store/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ const saveStateToLocalStorage = (state) => {
showFeatureHints: state.showFeatureHints,
flags: state.flags,
expandAllPipelines: state.expandAllPipelines,
display: state.display,
});

// Store Run's metadata to localstorage
Expand Down

0 comments on commit 46ee43e

Please sign in to comment.