-
Notifications
You must be signed in to change notification settings - Fork 172
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
Gird #712
Gird #712
Changes from 12 commits
6f24396
5f1e877
649d7c6
54c294c
374e5ba
862b353
663f3e0
0ef67f1
393547b
689ffdb
919ff9d
2270fc8
b81f9c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ | |
*/ | ||
|
||
import { List } from "immutable"; | ||
import { Dataset } from "plywood"; | ||
import { Dataset, Expression } from "plywood"; | ||
import * as React from "react"; | ||
import { Essence } from "../../../common/models/essence/essence"; | ||
import { FilterClause } from "../../../common/models/filter-clause/filter-clause"; | ||
|
@@ -109,8 +109,12 @@ export class BaseVisualization<S extends BaseVisualizationState> extends React.C | |
return this.debouncedCallExecutor(essence, timekeeper); | ||
} | ||
|
||
protected getQuery(essence: Essence, timekeeper: Timekeeper): Expression { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why this function only delegates to another function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is base/default implementation. Specific Visualisations can override this. And logic is extracted to function for better maintainability. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. clear, w need better abstraction for this in the future, just make a note for further work |
||
return makeQuery(essence, timekeeper); | ||
} | ||
|
||
private callExecutor = (essence: Essence, timekeeper: Timekeeper): Promise<DatasetLoad | null> => | ||
essence.dataCube.executor(makeQuery(essence, timekeeper), { timezone: essence.timezone }) | ||
essence.dataCube.executor(this.getQuery(essence, timekeeper), { timezone: essence.timezone }) | ||
.then((dataset: Dataset) => { | ||
// signal out of order requests with null | ||
if (!this.wasUsedForLastQuery(essence)) return null; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,153 @@ | ||
/* | ||
* Copyright 2017-2018 Allegro.pl | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import * as d3 from "d3"; | ||
import { Dataset, Datum, Expression, PseudoDatum } from "plywood"; | ||
import * as React from "react"; | ||
import { Essence } from "../../../common/models/essence/essence"; | ||
import { Timekeeper } from "../../../common/models/timekeeper/timekeeper"; | ||
import { Direction, ResizeHandle } from "../../components/resize-handle/resize-handle"; | ||
import { Scroller, ScrollerLayout } from "../../components/scroller/scroller"; | ||
import { selectFirstSplitDatums } from "../../utils/dataset/selectors/selectors"; | ||
import { BaseVisualization, BaseVisualizationState } from "../base-visualization/base-visualization"; | ||
import { FlattenedSplits } from "../table/body/splits/flattened-splits"; | ||
import { MeasuresHeader } from "../table/header/measures/measures-header"; | ||
import { SplitColumnsHeader } from "../table/header/splits/split-columns"; | ||
import { HEADER_HEIGHT, ROW_HEIGHT, SPACE_LEFT } from "../table/table"; | ||
import { measureColumnsCount } from "../table/utils/measure-columns-count"; | ||
import { visibleIndexRange } from "../table/utils/visible-index-range"; | ||
import makeQuery from "./make-query"; | ||
import { MeasureRows } from "./measure-rows"; | ||
|
||
interface GridState extends BaseVisualizationState { | ||
segmentWidth: number; | ||
} | ||
|
||
const MIN_DIMENSION_WIDTH = 100; | ||
const SEGMENT_WIDTH = 300; | ||
const MEASURE_WIDTH = 130; | ||
const SPACE_RIGHT = 10; | ||
|
||
export class Grid extends BaseVisualization<GridState> { | ||
protected innerGridRef = React.createRef<HTMLDivElement>(); | ||
|
||
protected getQuery(essence: Essence, timekeeper: Timekeeper): Expression { | ||
return makeQuery(essence, timekeeper); | ||
} | ||
|
||
getDefaultState(): GridState { | ||
return { | ||
segmentWidth: SEGMENT_WIDTH, | ||
...super.getDefaultState() | ||
}; | ||
} | ||
|
||
setScroll = (scrollTop: number, scrollLeft: number) => this.setState({ scrollLeft, scrollTop }); | ||
|
||
setSegmentWidth = (segmentWidth: number) => this.setState({ segmentWidth }); | ||
|
||
private getIdealColumnWidth(): number { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How to test calculation if method is private? I would extract layout math form the component class. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably you're right. As always, these functions mostly connect multiple variables from object state so tests will get messy. But extraction is good first step! |
||
const availableWidth = this.props.stage.width - SPACE_LEFT - this.getSegmentWidth(); | ||
const count = measureColumnsCount(this.props.essence); | ||
|
||
return count * MEASURE_WIDTH >= availableWidth ? MEASURE_WIDTH : availableWidth / count; | ||
} | ||
|
||
private getScalesForColumns(essence: Essence, flatData: PseudoDatum[]): Array<d3.scale.Linear<number, number>> { | ||
const concreteSeries = essence.getConcreteSeries().toArray(); | ||
|
||
return concreteSeries.map(series => { | ||
const measureValues = flatData | ||
.map((d: Datum) => series.selectValue(d)); | ||
|
||
return d3.scale.linear() | ||
// Ensure that 0 is in there | ||
.domain(d3.extent([0, ...measureValues])) | ||
.range([0, 100]); | ||
}); | ||
} | ||
|
||
maxSegmentWidth(): number { | ||
if (this.innerGridRef.current) { | ||
return this.innerGridRef.current.clientWidth - MIN_DIMENSION_WIDTH; | ||
} | ||
|
||
return SEGMENT_WIDTH; | ||
} | ||
|
||
getSegmentWidth(): number { | ||
const { segmentWidth } = this.state; | ||
return segmentWidth || SEGMENT_WIDTH; | ||
} | ||
|
||
protected renderInternals(dataset: Dataset): JSX.Element { | ||
const { essence, stage } = this.props; | ||
const { segmentWidth, scrollTop } = this.state; | ||
|
||
const data = selectFirstSplitDatums(dataset); | ||
|
||
const columnsCount = measureColumnsCount(essence); | ||
const columnWidth = this.getIdealColumnWidth(); | ||
const rowsCount = data.length; | ||
const visibleRowsRange = visibleIndexRange(rowsCount, stage.height, scrollTop); | ||
|
||
const layout: ScrollerLayout = { | ||
bodyWidth: columnWidth * columnsCount + SPACE_RIGHT, | ||
bodyHeight: rowsCount * ROW_HEIGHT, | ||
bottom: 0, | ||
left: this.getSegmentWidth(), | ||
right: 0, | ||
top: HEADER_HEIGHT | ||
}; | ||
|
||
return <div className="internals table table-inner" ref={this.innerGridRef}> | ||
<ResizeHandle | ||
direction={Direction.LEFT} | ||
onResize={this.setSegmentWidth} | ||
min={SEGMENT_WIDTH} | ||
max={this.maxSegmentWidth()} | ||
value={segmentWidth} | ||
/> | ||
<Scroller | ||
layout={layout} | ||
onScroll={this.setScroll} | ||
topGutter={<MeasuresHeader | ||
cellWidth={columnWidth} | ||
series={essence.getConcreteSeries().toArray()} | ||
commonSort={essence.getCommonSort()} | ||
showPrevious={essence.hasComparison()}/>} | ||
|
||
leftGutter={<FlattenedSplits | ||
visibleRowsIndexRange={visibleRowsRange} | ||
essence={essence} | ||
data={data} | ||
segmentWidth={segmentWidth} | ||
highlightedRowIndex={null} />} | ||
|
||
topLeftCorner={<SplitColumnsHeader essence={essence}/>} | ||
|
||
body={data && <MeasureRows | ||
visibleRowsIndexRange={visibleRowsRange} | ||
essence={essence} | ||
highlightedRowIndex={null} | ||
scales={this.getScalesForColumns(essence, data)} | ||
data={data} | ||
cellWidth={columnWidth} | ||
rowWidth={columnWidth * columnsCount} />} | ||
/> | ||
</div> ; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
/* | ||
* Copyright 2017-2018 Allegro.pl | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import { List } from "immutable"; | ||
import { $, Expression, LimitExpression, ply } from "plywood"; | ||
import { DataCube } from "../../../common/models/data-cube/data-cube"; | ||
import { Essence } from "../../../common/models/essence/essence"; | ||
import { ConcreteSeries } from "../../../common/models/series/concrete-series"; | ||
import { Sort } from "../../../common/models/sort/sort"; | ||
import { Split, toExpression as splitToExpression } from "../../../common/models/split/split"; | ||
import { TimeShiftEnv } from "../../../common/models/time-shift/time-shift-env"; | ||
import { Timekeeper } from "../../../common/models/timekeeper/timekeeper"; | ||
import { CANONICAL_LENGTH_ID } from "../../../common/utils/canonical-length/query"; | ||
import splitCanonicalLength from "../../../common/utils/canonical-length/split-canonical-length"; | ||
import timeFilterCanonicalLength from "../../../common/utils/canonical-length/time-filter-canonical-length"; | ||
import { assoc, thread } from "../../../common/utils/functional/functional"; | ||
import { SPLIT } from "../../config/constants"; | ||
|
||
const $main = $("main"); | ||
|
||
function applySeries(series: List<ConcreteSeries>, timeShiftEnv: TimeShiftEnv, nestingLevel = 0) { | ||
return (query: Expression) => { | ||
return series.reduce((query, series) => { | ||
return query.performAction(series.plywoodExpression(nestingLevel, timeShiftEnv)); | ||
}, query); | ||
}; | ||
} | ||
|
||
function applyLimit({ limit }: Split) { | ||
// TODO: this calculation is for evaluation purpose. We should add custom split values for Grid and remove this multiplication! | ||
const value = limit * 10; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. :) |
||
const limitExpression = new LimitExpression({ value }); | ||
return (query: Expression) => query.performAction(limitExpression); | ||
} | ||
|
||
function applySort(sort: Sort) { | ||
return (query: Expression) => query.performAction(sort.toExpression()); | ||
} | ||
|
||
function applyCanonicalLength(splits: List<Split>, dataCube: DataCube) { | ||
return (exp: Expression) => { | ||
const canonicalLength = splits | ||
.map(split => splitCanonicalLength(split, dataCube)) | ||
.filter(length => length !== null) | ||
.first(); | ||
if (!canonicalLength) return exp; | ||
return exp.apply(CANONICAL_LENGTH_ID, canonicalLength); | ||
}; | ||
} | ||
|
||
function applySplits(essence: Essence, timeShiftEnv: TimeShiftEnv): Expression { | ||
const { splits: { splits }, dataCube } = essence; | ||
const firstSplit = splits.first(); | ||
|
||
const splitsMap = splits.reduce<Record<string, Expression>>((map, split) => { | ||
const dimension = dataCube.getDimension(split.reference); | ||
const { name } = dimension; | ||
const expression = splitToExpression(split, dimension, timeShiftEnv); | ||
return assoc(map, name, expression); | ||
}, {}); | ||
|
||
return thread( | ||
$main.split(splitsMap), | ||
applyCanonicalLength(splits, dataCube), | ||
applySort(firstSplit.sort), | ||
applyLimit(firstSplit), | ||
applySeries(essence.getConcreteSeries(), timeShiftEnv) | ||
); | ||
} | ||
|
||
export default function makeQuery(essence: Essence, timekeeper: Timekeeper): Expression { | ||
const { splits, dataCube } = essence; | ||
if (splits.length() > dataCube.getMaxSplits()) throw new Error(`Too many splits in query. DataCube "${dataCube.name}" supports only ${dataCube.getMaxSplits()} splits`); | ||
|
||
const hasComparison = essence.hasComparison(); | ||
const mainFilter = essence.getEffectiveFilter(timekeeper, { combineWithPrevious: hasComparison }); | ||
|
||
const timeShiftEnv = essence.getTimeShiftEnv(timekeeper); | ||
|
||
const mainExp: Expression = ply() | ||
.apply("main", $main.filter(mainFilter.toExpression(dataCube))) | ||
.apply(CANONICAL_LENGTH_ID, timeFilterCanonicalLength(essence, timekeeper)); | ||
|
||
const queryWithMeasures = applySeries(essence.getConcreteSeries(), timeShiftEnv)(mainExp); | ||
|
||
return queryWithMeasures | ||
.apply(SPLIT, applySplits(essence, timeShiftEnv)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leaking abstraction, visualisation is responsible for "queryfn" so could be move this logic into visualisation itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, very leaking. These components are very far apart and don't have a good point to communicate. I knowingly made this change in the name of technical debt :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, just make a note and we will discuss how to define visualisation-make-query abstraction later on