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.

Sign up for GitHub

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

เกิดอยากลองทำ Heatmap ขึ้นมา #246

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

เกิดอยากลองทำ Heatmap ขึ้นมา #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
631bf30
Organize imports for PerPartyMapContainer
dtinth Mar 27, 2019
52ddc02
Implement Heatmap
dtinth Mar 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/components/ElectionMap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,10 +303,10 @@ class ElectionMap extends SvgChart {
// return match && match.complete ? 0 : this.options().size
}

opacity(d) {
return 1
const match = this.dataLookup[d.id]
return match && match.complete ? 1 : 0.5
opacity(data) {
const match = this.dataLookup[data.id]
return match && match.opacity != null ? match.opacity : 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The opacity computation should happen within this file, not the container. To separate visual encoding logic from data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm… I have to disagree (quite strongly) on this one, because as the ElectionMap gets re-used to display multiple aspects of the election result, the code in this file will become increasingly complex, trying to account for all the visualizations.

e.g.

  if (!match) {
    return 1
  }
  if (match.displayMode === 'heatmap') {
    if (match.areaStatus === 'winning') {
      return scaleLinear(/* ... */)
    } else if (match.areaStatus === 'sent') {
      return scaleLinear(/* ... */)
// (...so on...)

Not only the logic, but the interface would become more complex. for example, here’s what would happen to IMapZone to make election map have all the data necessary to compute the opacity:

 interface IMapZone {
   id: number
   partyId: number
   complete: boolean
   show: boolean
+  displayMode: 'heatmap' | 'winningCandidate'
+  areaStatus?: 'winning' | 'sent' | null
+  displayedCandidateScore?: number
+  winningCandidateScore?: number
+  totalVotersCount?: number
 }

Creating more visualizations would require editing code at 2 places — modifying the container to pass the required data to ElectionMap, and modifying the ElectionMap (which is 400+ lines already) to be able to make sense of that data. This shows a clear violation of the open-closed principle…

In fact, in my opinion, the ElectionMap would be easier to use and understand how did it become a more dumb component. i.e. it receives only what it needs to know to display it, and displays it, e.g.:

 interface IMapZone {
   id: number
-  partyId: number
-  complete: boolean
-  show: boolean
+  fill: 'solid' | 'striped'
+  color: string
+  opacity: number
 }

This would allow more visualization to be created just by creating a new container with little to no modification to the ElectionMap.

Copy link
Collaborator

@kristw kristw Mar 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough. agree with passing the computed encoding can be more convenient as the logic for computation is complex.

Another pattern that many d3-based components use is to pass

  • function color(datum){}
  • function opacity(datum){}
  • function color(datum){}
    as separate arguments
    (When using d3kit these can be passed via options)
    and pass data with all the necessary fields.

The pros of this approach are

  • The datum contains all raw fields. If there is any new encoding channel (e.g. strokeColor) that does not require new field in the raw data, only new encoding function is needed.
  • The datum is usually passed to the event listener, e.g. mouseenter that tooltip use. Having the raw data item can be convenient for display instead of having to perform lookup again.
  • matches general visualization mental model that map data to visual encoding channel.

The cons

  • precomputing the channels can be more optimized if they shared computation logic.

// return match && match.complete ? 1 : 0.5
}

resetZoom() {
Expand Down Expand Up @@ -375,7 +375,7 @@ class ElectionMap extends SvgChart {
.attr("transform", d => `translate(${d.x},${d.y})`)
.select("rect")
.attr("fill", d => this.color(d.data))
// .attr("opacity", d => this.opacity(d.data))
.attr("opacity", d => this.opacity(d.data))

zoneSelection.exit().remove()

Expand Down
113 changes: 84 additions & 29 deletions src/components/PerPartyMapContainer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,38 +1,27 @@
import React, { useCallback, useContext, useMemo, useState } from "react"
import {
checkFilter,
filters,
zones,
zonePath,
getZoneByProvinceIdAndZoneNo,
} from "../models/information"
import { useSummaryData } from "../models/LiveDataSubscription"
import _ from "lodash"
import React, { useCallback, useMemo } from "react"
import { getSeatDisplayModel } from "../models/ConstituencySeat"
import { zones, getPartyById } from "../models/information"
import { useSummaryData, usePerPartyData } from "../models/LiveDataSubscription"
import {
isZoneFinished,
shouldDisplayZoneData,
nationwidePartyStatsFromSummaryJSON,
} from "../models/PartyStats"
import { media, WIDE_NAV_MIN_WIDTH } from "../styles"
import ElectionMap, { electionMapLoadingData } from "./ElectionMap"
import ElectionMapTooltip from "./ElectionMapTooltip"
import ZoneMark from "./ZoneMark"
import { ZoneFilterContext } from "./ZoneFilterPanel"
import { navigate } from "gatsby"
import { trackEvent } from "../util/analytics"
import { media, WIDE_NAV_MIN_WIDTH } from "../styles"
import { getSeatDisplayModel } from "../models/ConstituencySeat"
import _ from "lodash"

/**
* @param {import('../models/LiveDataSubscription').DataState<ElectionDataSource.SummaryJSON>} summaryState
* @param {ElectionDataSource.SummaryJSON | null} summary
* @param {string} partyId
* @param {ReturnType<typeof computePartyCandidateModel>} perPartyModel
*/
function getMapData(summaryState, partyId) {
if (!summaryState.completed) {
function getMapData(summary, partyId, perPartyModel) {
if (!summary) {
return electionMapLoadingData
} else {
/** @type {ElectionDataSource.SummaryJSON} */
const summary = summaryState.data
const row = _.find(
nationwidePartyStatsFromSummaryJSON(summaryState.data),
nationwidePartyStatsFromSummaryJSON(summary),
row => row.party.id === +partyId
)
if (!row) return electionMapLoadingData
Expand All @@ -55,12 +44,40 @@ function getMapData(summaryState, partyId) {
}
return [
...zones.map((zone, i) => {
const { candidate, zoneStats } = getSeatDisplayModel(summary, zone)
const onMap = candidate && candidate.partyId === partyId
const { candidate: winningCandidate, zoneStats } = getSeatDisplayModel(
summary,
zone
)
const win = winningCandidate && +winningCandidate.partyId === +partyId
const sentCandidate = perPartyModel.getCandidate(
zone.provinceId,
zone.no
)
let opacity = 1
let winningPartyId = "nope"
let complete = false
const interpolate = (value, min = 0, max = 1) =>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use d3.scaleLinear or more specifically import { scaleLinear } from 'd3-scale' will save you many lines.

Also worth considering scaleQuantize instead to provide steps of colors instead of continuous range.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your suggestion — will do that.

Math.min(1, Math.max(0, (value - min) / (max - min)))
if (win) {
complete = true
winningPartyId = winningCandidate.partyId
opacity =
0.5 +
0.5 *
interpolate(
winningCandidate.score / zoneStats.votesTotal,
1 / 3,
1 / 2
)
} else if (sentCandidate && winningCandidate) {
winningPartyId = sentCandidate.partyId
opacity = interpolate(sentCandidate.score / winningCandidate.score)
}
return {
id: `${zone.provinceId}-${zone.no}`,
partyId: onMap ? candidate.partyId : "nope",
complete: onMap && isZoneFinished(zoneStats),
partyId: winningPartyId,
complete: complete,
opacity: opacity,
show: true,
}
}),
Expand All @@ -69,11 +86,43 @@ function getMapData(summaryState, partyId) {
}
}

/**
* @param {ElectionDataSource.PerPartyJSON | null} perPartyData
*/
function computePartyCandidateModel(perPartyData) {
const lookupTable = new Map(
perPartyData
? perPartyData.constituencyCandidates.map(candidate => [
`${candidate.provinceId}-${candidate.zone}`,
candidate,
])
: []
)
return {
/**
* @param {number} provinceId
* @param {number} zoneNo
* @return {ElectionDataSource.PerPartyCandidate | undefined}
*/
getCandidate(provinceId, zoneNo) {
return lookupTable.get(`${provinceId}-${zoneNo}`)
},
}
}

export default function PerPartyMapContainer({ partyId }) {
const summaryState = useSummaryData()
const perPartyState = usePerPartyData(partyId)
const party = getPartyById(partyId)
const partyCandidateModel = useMemo(
() => computePartyCandidateModel(perPartyState.data),
[perPartyState.data]
)
const mapData = useMemo(
() => ({ zones: getMapData(summaryState, partyId) }),
[summaryState, partyId]
() => ({
zones: getMapData(summaryState.data, partyId, partyCandidateModel),
}),
[summaryState.data, partyId, partyCandidateModel]
)

const onInit = useCallback(map => {}, [])
Expand All @@ -92,6 +141,12 @@ export default function PerPartyMapContainer({ partyId }) {
},
}}
>
<div style={{ textAlign: "center", marginBottom: 6 }}>
<ZoneMark color={party.color} />
มีผู้สมัครในเขตนั้น &nbsp;
<ZoneMark color={party.color} isCompleted />
ได้รับคะแนนสูงสุดในเขต
</div>
<ElectionMap
data={mapData}
onInit={onInit}
Expand Down
13 changes: 13 additions & 0 deletions src/models/ElectionDataSource.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,17 @@ declare namespace ElectionDataSource {
/** If result is overridden (see ResultOverride.js) */
overridden?: true
}

/**
* A file for each party which contains all necessary data to render
* the Per-Party view.
*/
interface PerPartyJSON {
updatedAt: DateString
constituencyCandidates: PerPartyCandidate[]
}
interface PerPartyCandidate extends Candidate {
provinceId: number
zone: number
}
}
9 changes: 9 additions & 0 deletions src/models/LiveDataSubscription.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,15 @@ export function usePerZoneData(provinceId, zoneNo) {
)
}

/** @return {DataState<ElectionDataSource.PerPartyJSON>} */
export function usePerPartyData(partyId) {
const state = useComputed(
() => getLatestDataFileState(`/PerPartyJSON/${partyId}.json`),
[partyId]
)
return useInertState(state)
}

function useMappedDataState(state, mapper) {
return useMemo(
() => (state.data ? { ...state, data: mapper(state.data) } : state),
Expand Down