Skip to content

Commit

Permalink
refactor: address PR review comments
Browse files Browse the repository at this point in the history
See #72
  • Loading branch information
evansiroky committed Aug 17, 2021
1 parent 69dd550 commit d4ecb7f
Show file tree
Hide file tree
Showing 12 changed files with 113 additions and 45 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// this file was autogenerated by tsdx using the `yarn lint --write-file` command
module.exports = {
"extends": [
"standard",
Expand Down
2 changes: 1 addition & 1 deletion lib/core/pattern.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export default class RoutePattern {
this.stops = []

// the inter-stop geometry, an array of point sequences (themselves arrays)
// that represent the geometry beween stops i and i+1. This array should be
// that represent the geometry between stops i and i+1. This array should be
// exactly one item shorter than the stops array.
this.interStopGeometry = []

Expand Down
34 changes: 24 additions & 10 deletions lib/display/display.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// The Display class is an abstract class and as such has some empty functions
/* eslint-disable @typescript-eslint/no-empty-function */

import LinearScale from '../util/linear-scale.js'

export default class Display {
Expand Down Expand Up @@ -102,6 +99,8 @@ export default class Display {

updateActiveZoomFactors(scale) {
let updated = false
console.log('scale: ', this.scale)
console.log('Last scale: ', this.lastScale)
for (let i = 0; i < this.zoomFactors.length; i++) {
const min = this.zoomFactors[i].minScale
const max =
Expand All @@ -125,9 +124,12 @@ export default class Display {
computeScale() {
this.lastScale = this.scale
this.scaleSet = true
const newXRes = (this.xDomain[1] - this.xDomain[0]) / this.width
const xDomainRange = this.xDomain[1] - this.xDomain[0]
const newXRes = xDomainRange / this.width
this.scale = this.initialXRes / newXRes
if (this.lastScale !== this.scale) this.scaleChanged()
console.log('Computed scale: ', this.xDomain)
console.log('Computed scale: ', this.scale)
}

scaleChanged() {
Expand Down Expand Up @@ -227,15 +229,27 @@ export default class Display {

/** Methods to be defined by subclasses **/

clear() {}
clear() {
throw new Error('method not implemented by subclass!')
}

drawCircle(coord, attrs) {}
drawCircle(coord, attrs) {
throw new Error('method not implemented by subclass!')
}

drawEllipse(coord, attrs) {}
drawEllipse(coord, attrs) {
throw new Error('method not implemented by subclass!')
}

drawRect(upperLeft, attrs) {}
drawRect(upperLeft, attrs) {
throw new Error('method not implemented by subclass!')
}

drawText(text, anchor, attrs) {}
drawText(text, anchor, attrs) {
throw new Error('method not implemented by subclass!')
}

drawPath(renderData, attrs) {}
drawPath(renderData, attrs) {
throw new Error('method not implemented by subclass!')
}
}
11 changes: 7 additions & 4 deletions lib/labeler/label.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// The Label class is an abstract class and as such has some empty functions
/* eslint-disable @typescript-eslint/no-empty-function */

/**
* Label object
*/
Expand All @@ -24,8 +21,14 @@ export default class Label {
: null
}

render(display) {}
render(display) {
throw new Error('method not defined by subclass!')
}

/**
* Does not need to be implemented by subclass
*/
// eslint-disable-next-line @typescript-eslint/no-empty-function
refresh(display) {}

setVisibility(visibility) {
Expand Down
10 changes: 0 additions & 10 deletions lib/point/place.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,4 @@ export default class Place extends Point {
y: this.renderXY.y - radius
}
}

/**
* Refresh the place
*
* @param {Display} display
*/

refresh(display) {
// FIXME is it ok this thing does nothing? FIXME added to avoid lint error.
}
}
35 changes: 29 additions & 6 deletions lib/point/point.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// The Point class is an abstract class and as such has some empty functions
/* eslint-disable @typescript-eslint/no-empty-function */

import { forEach } from 'lodash'

import PointLabel from '../labeler/pointlabel'
Expand Down Expand Up @@ -32,7 +29,9 @@ export default class Point {
* Get unique ID for point -- must be defined by subclass
*/

getId() {}
getId() {
throw new Error('method not defined by subclass!')
}

getElementId() {
return this.getType().toLowerCase() + '-' + this.getId()
Expand All @@ -42,7 +41,9 @@ export default class Point {
* Get Point type -- must be defined by subclass
*/

getType() {}
getType() {
throw new Error('method not defined by subclass!')
}

/**
* Get Point name
Expand Down Expand Up @@ -94,10 +95,20 @@ export default class Point {
* @param {Display} display
*/

render(display) {}
render(display) {
throw new Error('method not defined by subclass!')
}

/**
* Does not need to be implemented by subclass
*/
// eslint-disable-next-line @typescript-eslint/no-empty-function
addRenderData() {}

/**
* Does not need to be implemented by subclass
*/
// eslint-disable-next-line @typescript-eslint/no-empty-function
clearRenderData() {}

containsFromPoint() {
Expand Down Expand Up @@ -248,8 +259,16 @@ export default class Point {
return this.focused === true
}

/**
* Does not need to be implemented by subclass
*/
// eslint-disable-next-line @typescript-eslint/no-empty-function
runFocusTransition(display, callback) {}

/**
* Does not need to be implemented by subclass
*/
// eslint-disable-next-line @typescript-eslint/no-empty-function
setAllPatternsFocused() {}

getZIndex() {
Expand Down Expand Up @@ -277,6 +296,10 @@ export default class Point {
return dataArray && dataArray.length > 0
}

/**
* Does not need to be implemented by subclass
*/
// eslint-disable-next-line @typescript-eslint/no-empty-function
makeDraggable(transitive) {}

toString() {
Expand Down
4 changes: 2 additions & 2 deletions lib/renderer/default-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export default class DefaultRenderer extends Renderer {
})
})
} else {
// if we're returing to 'all-focused' mode
// if we're returning to 'all-focused' mode
// re-focus all internal points
forEach(graph.edges, (edge) => {
forEach(edge.pointArray, (point, i) => {
Expand Down Expand Up @@ -204,7 +204,7 @@ export default class DefaultRenderer extends Renderer {
.on('end', () => { if (!--n) this.transitive.refresh() })
}
// run the transtions on the affected elements
// run the transitions on the affected elements
forEach(focusChangeSegments, segment => {
segment.runFocusTransition(this.transitive.display, refreshOnEnd)
})
Expand Down
2 changes: 1 addition & 1 deletion lib/renderer/renderededge.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export default class RenderedEdge {

/**
* Computes the point of intersection between two adjacent, offset RenderedEdges (the
* edge the function is called on and a second egde passed as a parameter)
* edge the function is called on and a second edge passed as a parameter)
* by "extending" the adjacent edges and finding the point of intersection. If
* such a point exists, the existing renderData arrays for the edges are
* adjusted accordingly, as are any associated stops.
Expand Down
11 changes: 4 additions & 7 deletions lib/renderer/renderer.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// The Renderer class is an abstract class and as such has some empty functions
/* eslint-disable @typescript-eslint/no-empty-function */

import { forEach } from 'lodash'

/**
Expand Down Expand Up @@ -35,15 +32,15 @@ export default class Renderer {
}

/**
* sortElements
* Does not need to be implemented by subclass
*/

// eslint-disable-next-line @typescript-eslint/no-empty-function
sortElements() {}

/**
* focusPath
* Does not need to be implemented by subclass
*/

// eslint-disable-next-line @typescript-eslint/no-empty-function
focusPath(path) {}

isDraggable(point) {
Expand Down
2 changes: 1 addition & 1 deletion lib/styler/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const utils = {

return 'url(#' + markerId + ')'
},
fontSize: function (display, data) {
fontSize: function (display) {
return Math.floor(fontScale(display.scale))
},
pixels: function (zoom, min, normal, max) {
Expand Down
44 changes: 42 additions & 2 deletions lib/transitive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ type TransitiveData = {
stops: Stop[]
/**
* A list of street edges in the transportation network that are referenced by
* invididual journeys.
* individual journeys.
*/
streetEdges: StreetEdge[]
}
Expand Down Expand Up @@ -338,7 +338,47 @@ type TransitiveStyleComputeFn = (
* See code: https://github.com/conveyal/transitive.js/blob/6a8932930de003788b9034609697421731e7f812/lib/styler/styles.js#L17-L44
* FIXME add better typing once other files are typed
*/
styleUtils?: unknown
styleUtils?: {
/**
* Creates a svg definition for a marker and returns the string url for the
* defined marker
*/
defineSegmentCircleMarker: (
/**
* The display being used to render transitive
*/
display: unknown,
/**
* The segment to create a marker for
*/
segment: unknown,
/**
* The size of the radius that the marker should have
*/
radius: number,
/**
* The fill color of the marker
*/
fillColor: string
) => string
/**
* Calculates the font size based on the display's scale
*/
fontSize: (display: unknown) => number
/**
* Calculates something as it relates to the zoom in relation to zoom
*/
pixels: (
zoom: unknown,
min: unknown,
normal: unknown,
max: unknown
) => unknown
/**
* Calculates a stroke width depending on the display scale
*/
strokeWidth: (display: unknown) => unknown
}
) => number | string

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/util/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ function otpModeToGtfsType(otpMode) {
}
}

// Rendering utilties
// Rendering utilities

function renderDataToSvgPath(renderData) {
return renderData
Expand Down

0 comments on commit d4ecb7f

Please sign in to comment.