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] Auto-display legend in split mode + Fix legend and layer panel bugs #2190

Merged
merged 2 commits into from
Apr 11, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/src/common/checkbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const StyledRadiuInput = styled.label<StyledSwitchInputProps>`

const HiddenInput = styled.input`
position: absolute;
display: none;
opacity: 0;
`;

interface StyledCheckboxProps {
Expand Down
45 changes: 23 additions & 22 deletions src/components/src/common/data-table/grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,29 @@ import isEqual from 'lodash.isequal';
export default class GridHack extends PureComponent<GridProps> {
grid: Grid | null = null;

componentDidUpdate(preProps) {
/*
* This hack exists because in react-virtualized the
* _columnWidthGetter is only called in the constructor
* even though it is reassigned with new props resulting in
* a new width for cells not being calculated so we must
* force trigger a resize.
*
* https://github.com/bvaughn/react-virtualized/blob/master/source/Grid/Grid.js#L322
*
*/
if (!isEqual(preProps.cellSizeCache, this.props.cellSizeCache)) {
this.grid?.recomputeGridSize();
}
}

componentWillUnmount() {
//@ts-expect-error _scrollingContainer not typed in Grid
this.grid?._scrollingContainer?.removeEventListener('wheel', this._preventScrollBack, {
passive: false
});
}

_preventScrollBack = e => {
const {scrollLeft} = this.props;
if (scrollLeft !== undefined && scrollLeft <= 0 && e.deltaX < 0) {
Expand All @@ -50,28 +73,6 @@ export default class GridHack extends PureComponent<GridProps> {
});
}
};
componentWillUnmount() {
//@ts-expect-error _scrollingContainer not typed in Grid
this.grid?._scrollingContainer?.removeEventListener('wheel', this._preventScrollBack, {
passive: false
});
}

componentDidUpdate(preProps) {
/*
* This hack exists because in react-virtualized the
* _columnWidthGetter is only called in the constructor
* even though it is reassigned with new props resulting in
* a new width for cells not being calculated so we must
* force trigger a resize.
*
* https://github.com/bvaughn/react-virtualized/blob/master/source/Grid/Grid.js#L322
*
*/
if (!isEqual(preProps.cellSizeCache, this.props.cellSizeCache)) {
this.grid?.recomputeGridSize();
}
}

render() {
const {setGridRef, ...rest} = this.props;
Expand Down
40 changes: 36 additions & 4 deletions src/reducers/src/combined-updaters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,20 @@

import {
toggleModalUpdater,
loadFilesSuccessUpdater as uiStateLoadFilesSuccessUpdater
loadFilesSuccessUpdater as uiStateLoadFilesSuccessUpdater,
toggleMapControlUpdater,
toggleSplitMapUpdater as uiStateToggleSplitMapUpdater
} from './ui-state-updaters';
import {
updateVisDataUpdater as visStateUpdateVisDataUpdater,
setMapInfoUpdater,
layerTypeChangeUpdater
layerTypeChangeUpdater,
toggleSplitMapUpdater as visStateToggleSplitMapUpdater
} from './vis-state-updaters';
import {receiveMapConfigUpdater as stateMapConfigUpdater} from './map-state-updaters';
import {
receiveMapConfigUpdater as stateMapConfigUpdater,
toggleSplitMapUpdater as mapStateToggleSplitMapUpdater
} from './map-state-updaters';
import {
mapStyleChangeUpdater,
receiveMapConfigUpdater as styleMapConfigUpdater
Expand All @@ -40,7 +46,8 @@ import {ProviderState} from './provider-state-updaters';
import {
loadFilesSuccessUpdaterAction,
MapStyleChangeUpdaterAction,
LayerTypeChangeUpdaterAction
LayerTypeChangeUpdaterAction,
ToggleSplitMapUpdaterAction
} from '@kepler.gl/actions';
import {VisState} from '@kepler.gl/schemas';
import {Layer} from '@kepler.gl/layers';
Expand Down Expand Up @@ -345,3 +352,28 @@ export const combinedLayerTypeChangeUpdater = (
visState
};
};

/**
* Make mapLegend active when toggleSplitMap action is called
*/
export const toggleSplitMapUpdater = (
state: KeplerGlState,
action: ToggleSplitMapUpdaterAction
): KeplerGlState => {
const newState = {
...state,
visState: visStateToggleSplitMapUpdater(state.visState, action),
uiState: uiStateToggleSplitMapUpdater(state.uiState),
mapState: mapStateToggleSplitMapUpdater(state.mapState, action)
};

const isSplit = newState.visState.splitMaps.length !== 0;
const isLegendActive = newState.uiState.mapControls?.mapLegend?.active;
if (isSplit && !isLegendActive) {
newState.uiState = toggleMapControlUpdater(newState.uiState, {
payload: {panelId: 'mapLegend', index: action.payload}
});
}

return newState;
};
3 changes: 2 additions & 1 deletion src/reducers/src/composers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ const actionHandler = {
[ActionTypes.ADD_DATA_TO_MAP]: combinedUpdaters.addDataToMapUpdater,
[ActionTypes.MAP_STYLE_CHANGE]: combinedUpdaters.combinedMapStyleChangeUpdater,
[ActionTypes.LAYER_TYPE_CHANGE]: combinedUpdaters.combinedLayerTypeChangeUpdater,
[ActionTypes.LOAD_FILES_SUCCESS]: combinedUpdaters.loadFilesSuccessUpdater
[ActionTypes.LOAD_FILES_SUCCESS]: combinedUpdaters.loadFilesSuccessUpdater,
[ActionTypes.TOGGLE_SPLIT_MAP]: combinedUpdaters.toggleSplitMapUpdater
};

export default actionHandler;
8 changes: 1 addition & 7 deletions src/reducers/src/ui-state-updaters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,13 +381,7 @@ export const toggleMapControlUpdater = (
...state.mapControls,
[panelId]: {
...state.mapControls[panelId],
// this handles split map interaction
// Toggling from within the same map will simply toggle the active property
// Toggling from within different maps we set the active property to true
active:
index === state.mapControls[panelId].activeMapIndex
? !state.mapControls[panelId].active
: true,
active: !state.mapControls[panelId].active,
activeMapIndex: index
}
}
Expand Down
8 changes: 5 additions & 3 deletions test/browser/components/map/map-control-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ test('MapControlFactory - display options', t => {
children: <MapContainer {...propsWithSplitMap} />
});

t.equal(wrapper.find(MapControlButton).length, 6, 'Should show 6 MapControlButton');
// 5 control buttons as legend is opened automatically in split map mode
t.equal(wrapper.find(MapControlButton).length, 5, 'Should show 5 MapControlButton');
t.equal(wrapper.find(Split).length, 0, 'Should show 0 split map split button');
t.equal(wrapper.find(Delete).length, 1, 'Should show 1 split map delete button');
t.equal(wrapper.find(Layers).length, 1, 'Should show 1 Layer button');
Expand Down Expand Up @@ -121,9 +122,10 @@ test('MapControlFactory - click options', t => {
toggleMapControl: onToggleMapControl,
setLocale: onSetLocale
};
let updateState = keplerGlReducerCore(StateWSplitMaps, toggleMapControl('mapLegend', 0));
const mapContainerProps = mapFieldsSelector(
mockKeplerPropsWithState({
state: StateWSplitMaps,
state: updateState,
visStateActions,
mapStateActions,
uiStateActions
Expand All @@ -140,7 +142,7 @@ test('MapControlFactory - click options', t => {
}, 'MapContainer should not fail without props');

// layer selector is not active
t.equal(wrapper.find(MapControlButton).length, 6, 'Should show 5 MapControlButton');
t.equal(wrapper.find(MapControlButton).length, 6, 'Should show 6 MapControlButton');

t.equal(wrapper.find(Delete).length, 1, 'Should show 1 delete split map button');
// click split Map
Expand Down
2 changes: 2 additions & 0 deletions test/node/reducers/root-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,8 @@ test('keplerGlReducer - splitMap and mapControl interaction', t => {

t.equal(state.test.mapState.isSplit, true, 'Should have split map');

t.equal(state.test.uiState.mapControls.mapLegend.active, true, 'Should open map legend');

state = keplerGlReducer(state, toggleMapControl('mapDraw', 1));

t.equal(
Expand Down