Skip to content

Commit

Permalink
Allow linking settings on comparison pages (#519)
Browse files Browse the repository at this point in the history
Closes #504 

Mostly a refactor

Share the same link component on both article and comparison pages

Share the tests, but genericize them by base page
  • Loading branch information
lukebrody authored Nov 2, 2024
1 parent 8cd063b commit ff8ac89
Show file tree
Hide file tree
Showing 17 changed files with 205 additions and 196 deletions.
15 changes: 14 additions & 1 deletion react/src/components/QuerySettingsConnection.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { useContext, useEffect } from 'react'
import React, { ReactNode, useContext, useEffect } from 'react'

import { Settings, SettingsDictionary } from '../page_template/settings'
import { fromVector, useVector, VectorSettingKey } from '../page_template/settings-vector'
import { useAvailableGroups, useAvailableYears } from '../page_template/statistic-settings'

/**
* - Query Params -> Settings
Expand Down Expand Up @@ -49,3 +50,15 @@ export function QuerySettingsConnection({ settingsKeys }: { settingsKeys: Vector

return null
}

export function ArticleComparisonQuerySettingsConnection(): ReactNode {
const settingsKeys: VectorSettingKey[] = [
'use_imperial',
'show_historical_cds',
'simple_ordinals',
...useAvailableYears().map(year => `show_stat_year_${year}` as const),
...useAvailableGroups().map(group => `show_stat_group_${group.id}` as const),
]

return <QuerySettingsConnection settingsKeys={settingsKeys} />
}
19 changes: 3 additions & 16 deletions react/src/components/article-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@ import React, { ReactNode, useRef } from 'react'

import { article_link, comparison_link, sanitize } from '../navigation/links'
import { useSetting, useSettings } from '../page_template/settings'
import { VectorSettingKey } from '../page_template/settings-vector'
import { groupYearKeys, StatPathsContext, useAvailableGroups, useAvailableYears } from '../page_template/statistic-settings'
import { groupYearKeys, StatPathsContext } from '../page_template/statistic-settings'
import { PageTemplate } from '../page_template/template'
import { longname_is_exclusively_american, useUniverse } from '../universe'
import { Article, IRelatedButtons } from '../utils/protos'
import { useComparisonHeadStyle, useHeaderTextClass, useSubHeaderTextClass } from '../utils/responsive'
import { NormalizeProto } from '../utils/types'

import { ArticleWarnings } from './ArticleWarnings'
import { QuerySettingsConnection } from './QuerySettingsConnection'
import { ArticleComparisonQuerySettingsConnection } from './QuerySettingsConnection'
import { load_article } from './load-article'
import { Map } from './map'
import { Related } from './related-button'
Expand Down Expand Up @@ -45,7 +44,7 @@ export function ArticlePanel({ article }: { article: Article }): ReactNode {

return (
<StatPathsContext.Provider value={availableStatPaths}>
<ArticleQuerySettingsConnection />
<ArticleComparisonQuerySettingsConnection />
<PageTemplate screencap_elements={screencap_elements} has_universe_selector={true} universes={article.universes}>
<div>
<div ref={headers_ref}>
Expand Down Expand Up @@ -126,15 +125,3 @@ function StatisticRowHeader(): ReactNode {
const [simple_ordinals] = useSetting('simple_ordinals')
return <StatisticRowRaw index={0} _idx={-1} is_header={true} simple={simple_ordinals} />
}

function ArticleQuerySettingsConnection(): ReactNode {
const settingsKeys: VectorSettingKey[] = [
'use_imperial',
'show_historical_cds',
'simple_ordinals',
...useAvailableYears().map(year => `show_stat_year_${year}` as const),
...useAvailableGroups().map(group => `show_stat_group_${group.id}` as const),
]

return <QuerySettingsConnection settingsKeys={settingsKeys} />
}
2 changes: 2 additions & 0 deletions react/src/components/comparison-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { Article } from '../utils/protos'
import { useComparisonHeadStyle, useHeaderTextClass, useMobileLayout, useSubHeaderTextClass } from '../utils/responsive'

import { ArticleWarnings } from './ArticleWarnings'
import { ArticleComparisonQuerySettingsConnection } from './QuerySettingsConnection'
import { ArticleRow, load_article } from './load-article'
import { MapGeneric, MapGenericProps, Polygons } from './map'
import { WithPlot } from './plots'
Expand Down Expand Up @@ -135,6 +136,7 @@ export function ComparisonPanel(props: { joined_string: string, universes: strin

return (
<StatPathsContext.Provider value={Array.from(statPaths)}>
<ArticleComparisonQuerySettingsConnection />
<PageTemplate screencap_elements={screencap_elements} has_universe_selector={true} universes={props.universes}>
<div>
<div className={headerTextClass}>Comparison</div>
Expand Down
4 changes: 2 additions & 2 deletions react/test/article-universe_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ test('article-universe-compare', async (t) => {
.pressKey('enter')
await t.expect(getLocation())
.eql(
`${TARGET}/comparison.html?longnames=%5B%22San+Marino+city%2C+California%2C+USA%22%2C%22San+Francisco+city%2C+California%2C+USA%22%5D&universe=California%2C+USA`,
`${TARGET}/comparison.html?longnames=%5B%22San+Marino+city%2C+California%2C+USA%22%2C%22San+Francisco+city%2C+California%2C+USA%22%5D&universe=California%2C+USA&s=3t2X5xvsKo`,
)
await screencap(t)
})
Expand All @@ -107,7 +107,7 @@ test('article-universe-compare-different', async (t) => {
.pressKey('enter')
await t.expect(getLocation())
.eql(
`${TARGET}/comparison.html?longnames=%5B%22San+Marino+city%2C+California%2C+USA%22%2C%22Chicago+city%2C+Illinois%2C+USA%22%5D`,
`${TARGET}/comparison.html?longnames=%5B%22San+Marino+city%2C+California%2C+USA%22%2C%22Chicago+city%2C+Illinois%2C+USA%22%5D&s=3t2X5xvsKo`,
)
await screencap(t)
})
Expand Down
173 changes: 2 additions & 171 deletions react/test/article_link_settings_test.ts
Original file line number Diff line number Diff line change
@@ -1,172 +1,3 @@
import { Selector } from 'testcafe'
import { linkSettingsTests } from './link_settings_test_template'

import { arrayFromSelector, getLocation, screencap, TARGET, urbanstatsFixture } from './test_utils'

const baseLink = '/article.html?longname=California%2C+USA'

urbanstatsFixture('generate link', baseLink, async (t) => {
await t.click('.expandButton[data-category-id=main]')
})

const defaultLink = '/article.html?longname=California%2C+USA&s=3t2X5xvsKo'
const expectedLink = '/article.html?longname=California%2C+USA&s=jBXza8t6SU9'

test('formulates correct link', async (t) => {
// Check imperial, uncheck population
await t.click('input[data-test-id=use_imperial]')
await t.click('input[data-test-id=group_population]:not([inert] *)')

await t.expect(getLocation())
.eql(`${TARGET}${expectedLink}`)
})

urbanstatsFixture('paste link new visitor', expectedLink)

async function expectInputTestIdValues(t: TestController, mapping: Record<string, boolean>): Promise<void> {
for (const [testId, value] of Object.entries(mapping)) {
const selector = `input[data-test-id=${testId}]:not([inert] *)`
const isChecked = await Selector(selector).checked
await t.expect(isChecked).eql(value, `expected selector '${selector}' to have 'checked' value ${value}, but instead had ${isChecked}`)
}
}

test('settings are applied correctly to new visitor', async (t) => {
// assuming localstorage is cleared (happens in the fixture)
await t.click('.expandButton[data-category-id=main]')

// Should be no staging menu as this was first visit so we steal the settings from the vector
await t.expect(Selector('[data-test-id=staging_controls]').exists).notOk()

await expectInputTestIdValues(t, {
use_imperial: true,
group_population: false,
})

await screencap(t)
})

test('settings are not saved for new visitor if they do not make any modifications', async (t) => {
await t.navigateTo(baseLink)

await t.click('.expandButton[data-category-id=main]')

await expectInputTestIdValues(t, {
use_imperial: false,
group_population: true,
})

await t.expect(getLocation())
.eql(`${TARGET}${defaultLink}`)

await screencap(t)
})

test('settings are saved for new visitor if they do make a modification', async (t) => {
await t.click('input[data-test-id=year_2010]')

await t.navigateTo(baseLink)

await t.click('.expandButton[data-category-id=main]')

await expectInputTestIdValues(t, {
use_imperial: true,
group_population: false,
year_2010: true,
})

await screencap(t)
})

urbanstatsFixture('paste link previous visitor', baseLink, async (t) => {
await t.click('input[data-test-id=year_2010]') // change a setting so settings are saved
await t.navigateTo(expectedLink)
await t.click('.expandButton[data-category-id=main]')
})

async function expectHighlightedInputTestIds(t: TestController, testIds: string[]): Promise<void> {
const highlightedInputs = await arrayFromSelector(Selector('input[data-test-highlight=true]:not([inert] *)'))

await t.expect(await Promise.all(highlightedInputs.map(input => input.getAttribute('data-test-id')))).eql(testIds)
}

test('should have the staging controls', async (t) => {
await t.expect(Selector('[data-test-id=staging_controls]').exists).ok()

await expectHighlightedInputTestIds(t, ['use_imperial', 'year_2010', 'category_main', 'group_population'])

await screencap(t)
})

test('discard staged settings', async (t) => {
await t.click('button[data-test-id=discard]')
await t.expect(Selector('[data-test-id=staging_controls]').exists).notOk()
await expectHighlightedInputTestIds(t, [])
await expectInputTestIdValues(t, {
use_imperial: false,
group_population: true,
year_2010: true,
})

await screencap(t)
})

test('apply staged settings', async (t) => {
await t.click('button[data-test-id=apply]')
await t.expect(Selector('[data-test-id=staging_controls]').exists).notOk()
await expectHighlightedInputTestIds(t, [])
await expectInputTestIdValues(t, {
use_imperial: true,
group_population: false,
year_2010: false,
})

await t.eval(() => { location.reload() })

// Settings persist after reload without staging
await t.expect(Selector('[data-test-id=staging_controls]').exists).notOk()
await expectInputTestIdValues(t, {
use_imperial: true,
group_population: false,
year_2010: false,
})

await screencap(t)
})

test('manually discard changes', async (t) => {
await t.click('input[data-test-id=use_imperial]')
await t.click('input[data-test-id=group_population]:not([inert] *)')

await expectHighlightedInputTestIds(t, ['year_2010']) // category is unhighlighted because its groups aren't highlighted

await t.click('input[data-test-id=year_2010]')

await t.expect(Selector('[data-test-id=staging_controls]').exists).notOk()

await expectInputTestIdValues(t, {
use_imperial: false,
group_population: true,
year_2010: true,
})

await screencap(t)
})

test('apply some changes', async (t) => {
// Apply everything but use_imperial
await t.click('input[data-test-id=use_imperial]')

await expectHighlightedInputTestIds(t, ['year_2010', 'category_main', 'group_population'])

await t.click('button[data-test-id=apply]')

await t.expect(Selector('[data-test-id=staging_controls]').exists).notOk()

await expectInputTestIdValues(t, {
use_imperial: false,
group_population: false,
year_2010: false,
})

await screencap(t)
})
linkSettingsTests('/article.html?longname=California%2C+USA')
3 changes: 3 additions & 0 deletions react/test/comparison_link_settings_test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { linkSettingsTests } from './link_settings_test_template'

linkSettingsTests('/comparison.html?longnames=%5B%22Knoxville+%5BUrban+Area%5D%2C+TN%2C+USA%22%2C%22Louisville+KY+HSA%2C+Louisville+KY+HRR%2C+USA%22%5D')
Loading

0 comments on commit ff8ac89

Please sign in to comment.