diff --git a/react/src/components/QuerySettingsConnection.tsx b/react/src/components/QuerySettingsConnection.tsx index c2f6c0d2c..fbd38a3a7 100644 --- a/react/src/components/QuerySettingsConnection.tsx +++ b/react/src/components/QuerySettingsConnection.tsx @@ -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 @@ -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 +} diff --git a/react/src/components/article-panel.tsx b/react/src/components/article-panel.tsx index d18c48aa9..40c672f78 100644 --- a/react/src/components/article-panel.tsx +++ b/react/src/components/article-panel.tsx @@ -5,8 +5,7 @@ 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' @@ -14,7 +13,7 @@ import { useComparisonHeadStyle, useHeaderTextClass, useSubHeaderTextClass } fro 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' @@ -45,7 +44,7 @@ export function ArticlePanel({ article }: { article: Article }): ReactNode { return ( - +
@@ -126,15 +125,3 @@ function StatisticRowHeader(): ReactNode { const [simple_ordinals] = useSetting('simple_ordinals') return } - -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 -} diff --git a/react/src/components/comparison-panel.tsx b/react/src/components/comparison-panel.tsx index 56b9f5bb6..879f67d63 100644 --- a/react/src/components/comparison-panel.tsx +++ b/react/src/components/comparison-panel.tsx @@ -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' @@ -135,6 +136,7 @@ export function ComparisonPanel(props: { joined_string: string, universes: strin return ( +
Comparison
diff --git a/react/test/article-universe_test.ts b/react/test/article-universe_test.ts index a9ba42320..75d807c10 100644 --- a/react/test/article-universe_test.ts +++ b/react/test/article-universe_test.ts @@ -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) }) @@ -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) }) diff --git a/react/test/article_link_settings_test.ts b/react/test/article_link_settings_test.ts index 2d5b20270..0fa72c6a9 100644 --- a/react/test/article_link_settings_test.ts +++ b/react/test/article_link_settings_test.ts @@ -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): Promise { - 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 { - 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') diff --git a/react/test/comparison_link_settings_test.ts b/react/test/comparison_link_settings_test.ts new file mode 100644 index 000000000..a5d91b820 --- /dev/null +++ b/react/test/comparison_link_settings_test.ts @@ -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') diff --git a/react/test/link_settings_test_template.ts b/react/test/link_settings_test_template.ts new file mode 100644 index 000000000..7ad5d399b --- /dev/null +++ b/react/test/link_settings_test_template.ts @@ -0,0 +1,172 @@ +import { Selector } from 'testcafe' + +import { arrayFromSelector, getLocation, screencap, TARGET, urbanstatsFixture } from './test_utils' + +export function linkSettingsTests(baseLink: string): void { + urbanstatsFixture('generate link', baseLink, async (t) => { + await t.click('.expandButton[data-category-id=main]') + }) + + const defaultLink = `${baseLink}&s=3t2X5xvsKo` + const expectedLink = `${baseLink}&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): Promise { + 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 { + 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) + }) +} diff --git a/react/test/symlink_test.ts b/react/test/symlink_test.ts index 236ba5644..0ed6e8f9d 100644 --- a/react/test/symlink_test.ts +++ b/react/test/symlink_test.ts @@ -8,29 +8,29 @@ function symlink_test(name: string, link: string, expected: string | undefined = test(name, async (t) => { await screencap(t) - await t.expect(getLocation()).eql(`${TARGET}${expected}`) + await t.expect(getLocation()).eql(`${TARGET}${expected}&s=3t2X5xvsKo`) }) } symlink_test( 'national', '/article.html?longname=Timor-Leste', - '/article.html?longname=East+Timor&s=3t2X5xvsKo', + '/article.html?longname=East+Timor', ) symlink_test( 'subnational', '/article.html?longname=Haut-Lomami%2C+Congo%2C+The+Democratic+Republic+of+the', - '/article.html?longname=Haut-Lomami%2C+Democratic+Republic+of+the+Congo&s=3t2X5xvsKo', + '/article.html?longname=Haut-Lomami%2C+Democratic+Republic+of+the+Congo', ) symlink_test( 'urban center', '/article.html?longname=Tehr%C4%81n%2C+Iran%2C+Islamic+Republic+of', - '/article.html?longname=Tehr%C4%81n%2C+Iran&s=3t2X5xvsKo', + '/article.html?longname=Tehr%C4%81n%2C+Iran', ) symlink_test( '5mpc', '/article.html?longname=Pyongyang+5MPC%2C+Korea%2C+Democratic+People%27s+Republic+of', - '/article.html?longname=Pyongyang+5MPC%2C+North+Korea&s=3t2X5xvsKo', + '/article.html?longname=Pyongyang+5MPC%2C+North+Korea', ) symlink_test( @@ -42,7 +42,7 @@ symlink_test( symlink_test( 'usa-test', '/article.html?longname=United+States+of+America', - '/article.html?longname=USA&s=3t2X5xvsKo', + '/article.html?longname=USA', ) symlink_test('usa-comparison', '/comparison.html?longnames=%5B%22United+States+of+America%22%2C%22Canada%22%5D', '/comparison.html?longnames=%5B%22USA%22%2C%22Canada%22%5D') diff --git a/react/test/test_utils.ts b/react/test/test_utils.ts index c0c4d93fb..d7fa60156 100644 --- a/react/test/test_utils.ts +++ b/react/test/test_utils.ts @@ -13,6 +13,7 @@ export const IS_TESTING = true export function comparison_page(locations: string[]): string { const params = new URLSearchParams() params.set('longnames', JSON.stringify(locations)) + params.set('s', '3t2X5xvsKo') return `${TARGET}/comparison.html?${params.toString()}` } diff --git a/reference_test_screenshots/comparison_link_settings_test/Chrome/apply some changes-1.png b/reference_test_screenshots/comparison_link_settings_test/Chrome/apply some changes-1.png new file mode 100644 index 000000000..a6e4f52e9 Binary files /dev/null and b/reference_test_screenshots/comparison_link_settings_test/Chrome/apply some changes-1.png differ diff --git a/reference_test_screenshots/comparison_link_settings_test/Chrome/apply staged settings-1.png b/reference_test_screenshots/comparison_link_settings_test/Chrome/apply staged settings-1.png new file mode 100644 index 000000000..a1de1209b Binary files /dev/null and b/reference_test_screenshots/comparison_link_settings_test/Chrome/apply staged settings-1.png differ diff --git a/reference_test_screenshots/comparison_link_settings_test/Chrome/discard staged settings-1.png b/reference_test_screenshots/comparison_link_settings_test/Chrome/discard staged settings-1.png new file mode 100644 index 000000000..fca659503 Binary files /dev/null and b/reference_test_screenshots/comparison_link_settings_test/Chrome/discard staged settings-1.png differ diff --git a/reference_test_screenshots/comparison_link_settings_test/Chrome/manually discard changes-1.png b/reference_test_screenshots/comparison_link_settings_test/Chrome/manually discard changes-1.png new file mode 100644 index 000000000..fca659503 Binary files /dev/null and b/reference_test_screenshots/comparison_link_settings_test/Chrome/manually discard changes-1.png differ diff --git a/reference_test_screenshots/comparison_link_settings_test/Chrome/settings are applied correctly to new visitor-1.png b/reference_test_screenshots/comparison_link_settings_test/Chrome/settings are applied correctly to new visitor-1.png new file mode 100644 index 000000000..a1de1209b Binary files /dev/null and b/reference_test_screenshots/comparison_link_settings_test/Chrome/settings are applied correctly to new visitor-1.png differ diff --git a/reference_test_screenshots/comparison_link_settings_test/Chrome/settings are not saved for new visitor if they do not make any modifications-1.png b/reference_test_screenshots/comparison_link_settings_test/Chrome/settings are not saved for new visitor if they do not make any modifications-1.png new file mode 100644 index 000000000..0f9b8fe45 Binary files /dev/null and b/reference_test_screenshots/comparison_link_settings_test/Chrome/settings are not saved for new visitor if they do not make any modifications-1.png differ diff --git a/reference_test_screenshots/comparison_link_settings_test/Chrome/settings are saved for new visitor if they do make a modification-1.png b/reference_test_screenshots/comparison_link_settings_test/Chrome/settings are saved for new visitor if they do make a modification-1.png new file mode 100644 index 000000000..27ad23df5 Binary files /dev/null and b/reference_test_screenshots/comparison_link_settings_test/Chrome/settings are saved for new visitor if they do make a modification-1.png differ diff --git a/reference_test_screenshots/comparison_link_settings_test/Chrome/should have the staging controls-1.png b/reference_test_screenshots/comparison_link_settings_test/Chrome/should have the staging controls-1.png new file mode 100644 index 000000000..6e58ef98e Binary files /dev/null and b/reference_test_screenshots/comparison_link_settings_test/Chrome/should have the staging controls-1.png differ