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.

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

Feature/map tooltip #662

Open
wants to merge 4 commits into
base: staging
Choose a base branch
from
Open
Changes from 2 commits
Commits
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
56 changes: 49 additions & 7 deletions components/Map/Map.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const INITIAL_VIEW_STATE = {
const TOOLTIP_COMMON_STYLE = {
backgroundColor: colorTheme.newColors.black3,
borderRadius: '5px',
fontSize: '0.7em',
fontSize: '14px',
color: colorTheme.newColors.white,
}

Expand Down Expand Up @@ -98,6 +98,38 @@ const getColor = (
return colors[5]
}

const getColorFromDataPoint = (dataPoint: number | string): string => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This gets the job done, but would mean we have two independent definitions of the boundaries. I see there's a "boundaries" array as well. Could that be something to use?

let value: number;

if (typeof dataPoint === 'string') {
const standardizedDataPoint = dataPoint.trim().replace('−', '-').replace(',', '.');
value = parseFloat(standardizedDataPoint);
} else {
value = dataPoint;
}

if (value >= 0) {
return mapColors[0]; // 0%+
}
if (value < 0 && value >= -3) {
return mapColors[1]; // 0–3%
}
if (value < -3 && value >= -5) {
return mapColors[2]; // 3–5%
}
if (value < -5 && value >= -7) {
return mapColors[3]; // 5–7%
}
if (value < -7 && value >= -10) {
return mapColors[4]; // 7–10%
}
if (value < -10 && value >= -15) {
return mapColors[5]; // 10–15%
}

return mapColors[6];
};

// Use when viewState is reimplemented
/* const MAP_RANGE = {
lon: [8.107180004121693, 26.099158344940808],
Expand All @@ -117,6 +149,8 @@ export function isMunicipalityData(
}

function MobileTooltip({ tInfo }: { tInfo: MunicipalityTapInfo }) {
const dataPointColor = getColorFromDataPoint(tInfo.mData.dataPoint);
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this would be more readable inlined


return (
<Link
href={`/kommun/${tInfo.mData.name.toLowerCase()}`}
Expand All @@ -134,8 +168,8 @@ function MobileTooltip({ tInfo }: { tInfo: MunicipalityTapInfo }) {
color: colorTheme.newColors.white, height: 14, width: 14, marginRight: 4,
}}
/>
<span style={{ textDecoration: 'underline' }}>{`${tInfo.mData.name}`}</span>
{`: ${tInfo.mData.formattedDataPoint}`}
<span style={{ textDecoration: 'underline' }}>{`${tInfo.mData.name}`}:</span>
<span style={{ color: dataPointColor, fontWeight: 'bold' }}>{`${tInfo.mData.formattedDataPoint}%`}</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please run "npm run lint" to get a list of small nitpicks that will need to be addressed.

</Link>
)
}
Expand All @@ -146,7 +180,7 @@ function Map({
const [municipalityFeatureCollection, setMunicipalityFeatureCollection] = useState<any>({})
// "tapped" municipality tooltips are only to be used on touch devices.
const [lastTapInfo, setLastTapInfo] = useState<MunicipalityTapInfo | null>(null)
const wrapperRef = useRef<HTMLDivElement|null>(null)
const wrapperRef = useRef<HTMLDivElement | null>(null)

const router = useRouter()

Expand Down Expand Up @@ -264,11 +298,19 @@ function Map({
if (!isMunicipalityData(mData) || onTouchDevice()) {
return null // tooltips on touch devices are handled separately
}
return {

const dataPointColor = getColorFromDataPoint(mData.dataPoint);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to inline this variable


return ({
html: `
<p>${mData.name}: ${(mData).formattedDataPoint}</p>`,
<p>
${mData.name}:
<span style="color: ${dataPointColor}; font-weight: bold;">
${(mData).formattedDataPoint}%
</span>
</p>`,
style: TOOLTIP_COMMON_STYLE,
}
})
}}
onClick={({ object: mData, x, y }) => {
if (!isMunicipalityData(mData)) {
Expand Down