Skip to content

Commit

Permalink
Merge pull request #312 from flacoman91/expando-chart-bug
Browse files Browse the repository at this point in the history
fix expando chart bug
  • Loading branch information
flacoman91 authored Jul 14, 2020
2 parents 79332de + 6586189 commit 1c490e3
Show file tree
Hide file tree
Showing 16 changed files with 440 additions and 109 deletions.
18 changes: 15 additions & 3 deletions src/actions/__tests__/trends.spec.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { REQUERY_ALWAYS, REQUERY_NEVER } from '../../constants'
import * as sut from '../trends'
import { TREND_COLLAPSED, TREND_EXPANDED } from '../trends'

describe( 'action:trendsActions', () => {
describe( 'changeChartType', () => {
Expand Down Expand Up @@ -78,14 +79,25 @@ describe( 'action:trendsActions', () => {
} )
} )

describe( 'toggleTrend', () => {
describe( 'collapseTrend', () => {
it( 'creates a simple action', () => {
const expectedAction = {
type: sut.TREND_TOGGLED,
type: sut.TREND_COLLAPSED,
value: 'bar',
requery: REQUERY_NEVER
}
expect( sut.toggleTrend( 'bar' ) ).toEqual( expectedAction )
expect( sut.collapseTrend( 'bar' ) ).toEqual( expectedAction )
} )
} )

describe( 'expandTrend', () => {
it( 'creates a simple action', () => {
const expectedAction = {
type: sut.TREND_EXPANDED,
value: 'bar',
requery: REQUERY_NEVER
}
expect( sut.expandTrend( 'bar' ) ).toEqual( expectedAction )
} )
} )

Expand Down
24 changes: 20 additions & 4 deletions src/actions/trends.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ export const DATA_SUBLENS_CHANGED = 'DATA_SUBLENS_CHANGED'
export const DEPTH_CHANGED = 'DEPTH_CHANGED'
export const DEPTH_RESET = 'DEPTH_RESET'
export const FOCUS_CHANGED = 'FOCUS_CHANGED'
export const TREND_TOGGLED = 'TREND_TOGGLED'
export const TREND_COLLAPSED = 'TREND_COLLAPSED'
export const TREND_EXPANDED = 'TREND_EXPANDED'
export const TRENDS_TOOLTIP_CHANGED = 'TRENDS_TOOLTIP_CHANGED'

/**
Expand Down Expand Up @@ -108,15 +109,30 @@ export function updateTrendsTooltip( value ) {
}

/**
* Indicates a bar in row chart has been toggled
* Indicates a bar in row chart has been collapsed
*
* @param {string} value of trend agg that was toggled
* @returns {string} a packaged payload to be used by Redux reducers
*/
export function toggleTrend( value ) {
export function collapseTrend( value ) {
return {
type: TREND_TOGGLED,
type: TREND_COLLAPSED,
requery: REQUERY_NEVER,
value
}
}

/**
* Indicates a bar in row chart has been expanded
*
* @param {string} value of trend agg that was toggled
* @returns {string} a packaged payload to be used by Redux reducers
*/
export function expandTrend( value ) {
return {
type: TREND_EXPANDED,
requery: REQUERY_NEVER,
value
}
}

34 changes: 28 additions & 6 deletions src/components/Charts/RowChart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import './RowChart.less'
import * as d3 from 'd3'
import { changeFocus, toggleTrend } from '../../actions/trends'
import { changeFocus, collapseTrend, expandTrend } from '../../actions/trends'
import { miniTooltip, row } from 'britecharts'
import { connect } from 'react-redux'
import { hashObject } from '../../utils'
Expand All @@ -16,6 +16,7 @@ export class RowChart extends React.Component {
constructor( props ) {
super( props )
this._selectFocus = this._selectFocus.bind( this )
this._toggleRow = this._toggleRow.bind( this )
}

_formatTip( value ) {
Expand Down Expand Up @@ -87,7 +88,7 @@ export class RowChart extends React.Component {
// eslint-disable-next-line complexity
_redrawChart() {
const {
colorScheme, data, id, printMode, toggleRow, total
colorScheme, data, id, printMode, total
} = this.props
// deep copy
// do this to prevent REDUX pollution
Expand Down Expand Up @@ -153,7 +154,7 @@ export class RowChart extends React.Component {

rowContainer
.selectAll( '.y-axis-group .tick' )
.on( 'click', toggleRow )
.on( 'click', this._toggleRow )

rowContainer
.selectAll( '.view-more-label' )
Expand All @@ -167,6 +168,19 @@ export class RowChart extends React.Component {
this.props.selectFocus( element, lens )
}

_toggleRow( rowName ) {
// fire off different action depending on if the row is expanded or not
const { expandableRows, expandedTrends } = this.props
if ( expandableRows.includes( rowName ) ) {
if ( expandedTrends.includes( rowName ) ) {
this.props.collapseRow( rowName )
} else {
this.props.expandRow( rowName )
}
}
}


render() {
return (
this.props.total > 0 &&
Expand All @@ -185,18 +199,26 @@ export const mapDispatchToProps = dispatch => ( {
scrollToFocus()
dispatch( changeFocus( element.parent, lens ) )
},
toggleRow: selectedState => {
dispatch( toggleTrend( selectedState ) )
collapseRow: rowName => {
dispatch( collapseTrend( rowName.trim() ) )
},
expandRow: rowName => {
dispatch( expandTrend( rowName.trim() ) )
}
} )

export const mapStateToProps = state => {
const lens = state.query.tab === MODE_MAP ? 'Product' : state.query.lens
const { tab } = state.query
const lens = tab === MODE_MAP ? 'Product' : state.query.lens
const { expandableRows, expandedTrends } = state[tab.toLowerCase()]
const { printMode, showTrends, width } = state.view
return {
expandableRows,
expandedTrends,
lens,
printMode,
showTrends,
tab,
width
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/components/Print/print.less
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@
.u-right,
.total,
#clear-focus,
.trend-depth-toggle {
.trend-depth-toggle,
.view-more-label {
display: none !important;
}

Expand Down Expand Up @@ -94,7 +95,8 @@
.a-micro-copy,
footer,
#clear-focus,
.trend-depth-toggle {
.trend-depth-toggle,
.view-more-label {
display: none !important;
}
}
6 changes: 4 additions & 2 deletions src/components/__tests__/MapPanel.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import ReduxMapPanel, { MapPanel, mapDispatchToProps } from '../Map/MapPanel'
import React from 'react'
import renderer from 'react-test-renderer'
import thunk from 'redux-thunk'
import { MODE_MAP } from '../../constants'

function setupSnapshot( { enablePer1000, printMode } ) {
const items = [
Expand All @@ -28,12 +29,13 @@ function setupSnapshot( { enablePer1000, printMode } ) {
}
},
query: {
date_received_min: new Date('7/10/2017'),
date_received_max: new Date('7/10/2020'),
enablePer1000,
mapWarningEnabled: true,
issue: [],
product: [],
date_received_min: new Date('7/10/2017'),
date_received_max: new Date('7/10/2020')
tab: MODE_MAP
},
view: {
printMode,
Expand Down
103 changes: 99 additions & 4 deletions src/components/__tests__/RowChart.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,69 @@ describe( 'component: RowChart', () => {
expect( cb ).toHaveBeenCalledTimes( 1 )
expect( cb ).toHaveBeenCalledWith( { name: 'foo' }, 'Product' )
} )

describe( 'row toggles', () => {
let expandCb, collapseCb
beforeEach( () => {
collapseCb = jest.fn()
expandCb = jest.fn()
} )
it( 'ignores values not in expandable rows', () => {
const target = shallow( <RowChart
lens={ 'Overview' }
collapseRow={ collapseCb }
expandRow={ expandCb }
colorScheme={ [] }
title={ 'test' }
data={ [ 23, 4, 3 ] }
expandableRows={ [ 'foo', 'bar' ] }
expandedTrends={ [ 'foo' ] }
id={ 'foo' }
total={ 1000 }
/> )
target.instance()._toggleRow( 'not a expandable row' )
expect( collapseCb ).toHaveBeenCalledTimes( 0 )
expect( expandCb ).toHaveBeenCalledTimes( 0 )
} )

it( 'collapses a row', () => {
const target = shallow( <RowChart
lens={ 'Overview' }
collapseRow={ collapseCb }
expandRow={ expandCb }
colorScheme={ [] }
title={ 'test' }
data={ [ 23, 4, 3 ] }
expandableRows={ [ 'foo', 'bar' ] }
expandedTrends={ [ 'foo' ] }
id={ 'foo' }
total={ 1000 }
/> )
target.instance()._toggleRow( 'foo' )
expect( collapseCb ).toHaveBeenCalledTimes( 1 )
expect( collapseCb ).toHaveBeenCalledWith( 'foo' )
expect( expandCb ).toHaveBeenCalledTimes( 0 )
} )

it( 'expands a row', () => {
const target = shallow( <RowChart
lens={ 'Overview' }
collapseRow={ collapseCb }
expandRow={ expandCb }
colorScheme={ [] }
title={ 'test' }
data={ [ 23, 4, 3 ] }
expandableRows={ [ 'foo', 'bar' ] }
expandedTrends={ [] }
id={ 'foo' }
total={ 1000 }
/> )
target.instance()._toggleRow( 'foo' )
expect( expandCb ).toHaveBeenCalledTimes( 1 )
expect( expandCb ).toHaveBeenCalledWith( 'foo' )
expect( collapseCb ).toHaveBeenCalledTimes( 0 )
} )
} )
} )

describe( 'mapDispatchToProps', () => {
Expand All @@ -236,10 +299,25 @@ describe( 'component: RowChart', () => {
expect( trendsUtils.scrollToFocus ).toHaveBeenCalled()
} )

it( 'hooks into toggleTrend', () => {
it( 'hooks into collapseTrend', () => {
spyOn( trendsUtils, 'scrollToFocus' )
mapDispatchToProps( dispatch ).collapseRow( 'Some Expanded row' )
expect( dispatch.mock.calls ).toEqual( [ [ {
requery: 'REQUERY_NEVER',
type: 'TREND_COLLAPSED',
value: 'Some Expanded row'
} ] ] )
expect( trendsUtils.scrollToFocus ).not.toHaveBeenCalled()
} )

it( 'hooks into expandTrend', () => {
spyOn( trendsUtils, 'scrollToFocus' )
mapDispatchToProps( dispatch ).toggleRow()
expect( dispatch.mock.calls.length ).toEqual( 1 )
mapDispatchToProps( dispatch ).expandRow( 'collapse row name' )
expect( dispatch.mock.calls ).toEqual( [ [ {
requery: 'REQUERY_NEVER',
type: 'TREND_EXPANDED',
value: 'collapse row name'
} ] ] )
expect( trendsUtils.scrollToFocus ).not.toHaveBeenCalled()
} )
} )
Expand All @@ -248,12 +326,21 @@ describe( 'component: RowChart', () => {
let state
beforeEach( () => {
state = {
map: {
expandableRows: [],
expandedTrends: []
},
query: {
lens: 'Foo',
tab: 'Map'
},
trends: {
expandableRows: [],
expandedTrends: []
},
view: {
printMode: false,
showTrends: true,
width: 1000
}
}
Expand All @@ -264,8 +351,12 @@ describe( 'component: RowChart', () => {
}
let actual = mapStateToProps( state, ownProps )
expect( actual ).toEqual( {
expandableRows: [],
expandedTrends: [],
lens: 'Product',
printMode: false,
showTrends: true,
tab: 'Map',
width: 1000
} )
} )
Expand All @@ -275,12 +366,16 @@ describe( 'component: RowChart', () => {
id: 'baz'
}

state.query.tab = 'Bar'
state.query.tab = 'Trends'

let actual = mapStateToProps( state, ownProps )
expect( actual ).toEqual( {
expandableRows: [],
expandedTrends: [],
lens: 'Foo',
printMode: false,
tab: 'Trends',
showTrends: true,
width: 1000
} )
} )
Expand Down
2 changes: 2 additions & 0 deletions src/components/__tests__/TrendsPanel.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import ReduxTrendsPanel, {
TrendsPanel,
mapDispatchToProps
} from '../Trends/TrendsPanel'
import { MODE_TRENDS } from '../../constants'
import React from 'react'
import renderer from 'react-test-renderer'
import { shallow } from 'enzyme'
Expand Down Expand Up @@ -89,6 +90,7 @@ function setupSnapshot( { chartType,
date_received_max: "2020-01-01T00:00:00.000Z",
lens,
subLens,
tab: MODE_TRENDS,
trendsDateWarningEnabled
},
trends: {
Expand Down
2 changes: 1 addition & 1 deletion src/reducers/__fixtures__/trendsAggsDupes.jsx

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/reducers/__fixtures__/trendsAggsMissingBuckets.jsx

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/reducers/__fixtures__/trendsBackfill.jsx

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/reducers/__fixtures__/trendsFocusAggs.jsx

Large diffs are not rendered by default.

Loading

0 comments on commit 1c490e3

Please sign in to comment.