Merge pull request #8 from theia-ide/jk/review

code review

timeline-chart/src/components/time-graph-grid.ts

@@ -17,17 +17,5 @@ export class TimeGraphGrid extends TimeGraphAxisScale {
 
     render(): void {
         this.renderVerticalLines(this.stateController.canvasDisplayHeight, 0xdddddd);
-
-        // const rowNumber = Math.trunc(this.stateController.canvasDisplayHeight / this.rowHeight) + 2;
-        // for (let i = 0; i < rowNumber; i++) {
-        //     this.hline({
-        //         color: 0xdddddd,
-        //         position: {
-        //             x: this._options.position.x,
-        //             y: (i * this.rowHeight) - (this.rowHeight / 2)
-        //         },
-        //         width: this._options.width
-        //     });
-        // }
     }
 }

timeline-chart/src/layer/time-graph-chart-arrows.ts

@@ -44,17 +44,6 @@ export class TimeGraphChartArrows extends TimeGraphChartLayer {
     }
 
     protected update(): void {
-        // PIXI.loader
-        //     .add(ArrowHead)
-        //     .load((() => {
-        //     let sprite = new PIXI.Sprite(
-        //         PIXI.loader.resources[ArrowHead].texture
-        //     );
-        //     sprite.x = 100;
-        //     sprite.y = 100;
-        //     this.stage.addChild(sprite);
-        // }).bind(this));
-
         if (this.arrows) {
             this.removeChildren();
             this.addArrows(this.arrows);

timeline-chart/src/layer/time-graph-chart.ts

@@ -35,8 +35,7 @@ export class TimeGraphChart extends TimeGraphChartLayer {
 
     constructor(id: string,
         protected providers: TimeGraphChartProviders,
-        protected rowController: TimeGraphRowController,
-        protected viewRange?: TimeGraphRange) {
+        protected rowController: TimeGraphRowController) {
         super(id, rowController);
         this.providedRange = { start: 0, end: 0 };
         this.providedResolution = 1;
@@ -60,12 +59,8 @@ export class TimeGraphChart extends TimeGraphChartLayer {
             this.layer.position.y = -verticalOffset;
         });
 
-        if (!this.viewRange) {
-            this.viewRange = this.unitController.viewRange;
-        }
         this.unitController.onViewRangeChanged(() => {
             this.updateScaleAndPosition();
-            this.viewRange = this.unitController.viewRange;
             if (!this.fetching) {
                 this.maybeFetchNewData();
             }
@@ -78,13 +73,14 @@ export class TimeGraphChart extends TimeGraphChartLayer {
 
     protected maybeFetchNewData() {
         const resolution = this.unitController.viewRangeLength / this.stateController.canvasDisplayWidth;
-        if (this.viewRange && (
-            this.viewRange.start < this.providedRange.start ||
-            this.viewRange.end > this.providedRange.end ||
+        const viewRange = this.unitController.viewRange;
+        if (viewRange && (
+            viewRange.start < this.providedRange.start ||
+            viewRange.end > this.providedRange.end ||
             resolution < this.providedResolution
         )) {
             this.fetching = true;
-            const rowData = this.providers.dataProvider(this.viewRange, resolution);
+            const rowData = this.providers.dataProvider(viewRange, resolution);
             if (rowData) {
                 this.providedResolution = rowData.resolution;
                 this.providedRange = rowData.range;

timeline-chart/src/time-graph-model.ts

@@ -5,6 +5,7 @@ export interface TimeGraphRange {
 
 export interface TimeGraphModel {
     id: string
+    // what is this? A range is usually an interval with start and end. Consider rename
     totalRange: number
     rows: TimeGraphRowModel[]
     arrows: TimeGraphArrow[]
@@ -32,4 +33,5 @@ export interface TimeGraphArrow {
     destinationId: number
     range: TimeGraphRange
     data?: {[key:string]:any}
+    // Q: Can I select an arrow?
 }

timeline-chart/src/time-graph-row-controller.ts

@@ -3,27 +3,27 @@ import { TimeGraphRowModel } from "./time-graph-model";
 export class TimeGraphRowController {
     private _selectedRow: TimeGraphRowModel;
     private _verticalOffset: number;
-    protected selectedRowChangedHandler: ((row: TimeGraphRowModel) => void)[] = [];
-    protected verticalOffsetChangedHandler: ((verticalOffset: number) => void)[] = [];
+    protected selectedRowChangedHandlers: ((row: TimeGraphRowModel) => void)[] = [];
+    protected verticalOffsetChangedHandlers: ((verticalOffset: number) => void)[] = [];
 
     constructor(public rowHeight: number, public totalHeight: number) {
         this._verticalOffset = 0;
     }
 
     protected handleVerticalOffsetChanged(){
-        this.verticalOffsetChangedHandler.forEach(h=>h(this._verticalOffset));
+        this.verticalOffsetChangedHandlers.forEach(h=>h(this._verticalOffset));
     }
 
     protected handleSelectedRowChanged(){
-        this.selectedRowChangedHandler.forEach(h=>h(this._selectedRow));
+        this.selectedRowChangedHandlers.forEach(h=>h(this._selectedRow));
     }
 
     onSelectedRowChangedHandler(handler: (row: TimeGraphRowModel) => void) {
-        this.selectedRowChangedHandler.push(handler);
+        this.selectedRowChangedHandlers.push(handler);
     }
 
     onVerticalOffsetChangedHandler(handler: (verticalOffset: number) => void) {
-        this.verticalOffsetChangedHandler.push(handler);
+        this.verticalOffsetChangedHandlers.push(handler);
     }
 
     get verticalOffset(): number {

timeline-chart/src/time-graph-state-controller.ts

@@ -21,8 +21,8 @@ export class TimeGraphStateController {
         y: number;
     };
 
-    protected zoomChangedHandler: (() => void)[];
-    protected positionChangedHandler: (() => void)[];
+    protected zoomChangedHandlers: (() => void)[];
+    protected positionChangedHandlers: (() => void)[];
 
     constructor(protected canvas: HTMLCanvasElement, protected unitController: TimeGraphUnitController) {
         const ratio = window.devicePixelRatio;
@@ -31,22 +31,22 @@ export class TimeGraphStateController {
         this._initialZoomFactor = this.zoomFactor;
         this._positionOffset = { x: 0, y: 0 };
         this.oldPositionOffset = { x: 0, y: 0 };
-        this.zoomChangedHandler = [];
-        this.positionChangedHandler = [];
+        this.zoomChangedHandlers = [];
+        this.positionChangedHandlers = [];
     }
 
     protected handleZoomChange() {
-        this.zoomChangedHandler.map(handler => handler());
+        this.zoomChangedHandlers.forEach(handler => handler());
     }
     protected handlePositionChange() {
-        this.positionChangedHandler.map(handler => handler());
+        this.positionChangedHandlers.forEach(handler => handler());
     }
 
     onZoomChanged(handler: () => void) {
-        this.zoomChangedHandler.push(handler);
+        this.zoomChangedHandlers.push(handler);
     }
     onPositionChanged(handler: () => void) {
-        this.positionChangedHandler.push(handler);
+        this.positionChangedHandlers.push(handler);
     }
 
     get initialZoomFactor(): number {

timeline-chart/src/time-graph-unit-controller.ts

@@ -1,35 +1,35 @@
 import { TimeGraphRange } from "./time-graph-model";
 
 export class TimeGraphUnitController {
-    protected viewRangeChangedHandler: ((newRange: TimeGraphRange) => void)[];
+    protected viewRangeChangedHandlers: ((newRange: TimeGraphRange) => void)[];
     protected _viewRange: TimeGraphRange;
 
-    protected selectionRangeChangedHandler: ((newRange?: TimeGraphRange) => void)[];
+    protected selectionRangeChangedHandlers: ((newRange?: TimeGraphRange) => void)[];
     protected _selectionRange?: TimeGraphRange;
 
     numberTranslator?: (theNumber: number) => string;
 
     constructor(public absoluteRange: number, viewRange?: TimeGraphRange) {
-        this.viewRangeChangedHandler = [];
+        this.viewRangeChangedHandlers = [];
         this._viewRange = viewRange || { start: 0, end: absoluteRange };
 
-        this.selectionRangeChangedHandler = [];
+        this.selectionRangeChangedHandlers = [];
     }
 
     protected handleViewRangeChange() {
-        this.viewRangeChangedHandler.map(handler => handler(this._viewRange));
+        this.viewRangeChangedHandlers.forEach(handler => handler(this._viewRange));
     }
 
     protected handleSelectionRangeChange() {
-        this.selectionRangeChangedHandler.map(handler => handler(this._selectionRange));
+        this.selectionRangeChangedHandlers.forEach(handler => handler(this._selectionRange));
     }
 
     onViewRangeChanged(handler: (viewRange: TimeGraphRange) => void) {
-        this.viewRangeChangedHandler.push(handler);
+        this.viewRangeChangedHandlers.push(handler);
     }
 
     onSelectionRangeChange(handler: (selectionRange: TimeGraphRange) => void) {
-        this.selectionRangeChangedHandler.push(handler);
+        this.selectionRangeChangedHandlers.push(handler);
     }
 
     get viewRange(): TimeGraphRange {