From f9e82ba9afc0176d6b72ce6f4ddad6b80e71a94a Mon Sep 17 00:00:00 2001 From: james hadfield Date: Thu, 30 Jan 2020 15:58:45 +1300 Subject: [PATCH 1/7] avoid duplicate react keys We can use array indexes as keys because the arrays are not modified. --- src/components/narrative/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/narrative/index.js b/src/components/narrative/index.js index c357bb03a..6842b54de 100644 --- a/src/components/narrative/index.js +++ b/src/components/narrative/index.js @@ -1,4 +1,5 @@ /* eslint-disable react/no-danger */ +/* eslint-disable react/no-array-index-key */ import React from "react"; import { connect } from "react-redux"; import queryString from "query-string"; @@ -114,7 +115,7 @@ class Narrative extends React.Component { const d = (!this.state.showingEndOfNarrativePage) && this.props.currentInFocusBlockIdx === i ? "14px" : "6px"; return (
this.reactPageScroller.goToPage(i)} />); @@ -126,7 +127,7 @@ class Narrative extends React.Component { const ret = this.props.blocks.map((b, i) => (
Date: Thu, 30 Jan 2020 16:05:19 +1300 Subject: [PATCH 2/7] add comments & remove unnecessary console log --- src/actions/recomputeReduxState.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index 15a7e4fcb..22a0c0a69 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -612,12 +612,17 @@ export const createStateFromQueryOrJSONs = ({ controls = restoreQueryableStateToDefaults(controls); } + + /* For the creation of state, we want to parse out URL query parameters + (e.g. ?c=country means we want to color-by country) and modify the state + accordingly. For narratives, we _don't_ display these in the URL, instead + only displaying the page number (e.g. ?n=3), but we can look up what (hidden) + URL query this page defines via this information */ if (narrativeBlocks) { narrative = narrativeBlocks; const n = parseInt(query.n, 10) || 0; controls = modifyStateViaURLQuery(controls, queryString.parse(narrative[n].query)); query = {n}; // eslint-disable-line - console.log("redux state changed to relfect n of", n); } else { controls = modifyStateViaURLQuery(controls, query); } From bc9c23a19c51719eb0cf783ad5d4c62fdd9b900e Mon Sep 17 00:00:00 2001 From: james hadfield Date: Thu, 30 Jan 2020 16:17:32 +1300 Subject: [PATCH 3/7] [narrative bugfix] URL query now updates selected sidebar page --- src/reducers/narrative.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/reducers/narrative.js b/src/reducers/narrative.js index 05fdee164..a207e4b26 100644 --- a/src/reducers/narrative.js +++ b/src/reducers/narrative.js @@ -1,4 +1,3 @@ -import queryString from "query-string"; import * as types from "../actions/types"; const explanationParagraph=` @@ -32,7 +31,7 @@ const narrative = (state = { blocks, title: blocks[0].__html.match(/>(.+?) Date: Thu, 30 Jan 2020 16:27:05 +1300 Subject: [PATCH 4/7] don't show n=0 URL query for narratives If a n query parameter is not set for a narrative it's implicit to start from the first page, we don't need to add `?n=0` --- src/actions/recomputeReduxState.js | 2 +- src/middleware/changeURL.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index 22a0c0a69..bd5e6b3b3 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -622,7 +622,7 @@ export const createStateFromQueryOrJSONs = ({ narrative = narrativeBlocks; const n = parseInt(query.n, 10) || 0; controls = modifyStateViaURLQuery(controls, queryString.parse(narrative[n].query)); - query = {n}; // eslint-disable-line + query = n===0 ? {} : {n}; // eslint-disable-line } else { controls = modifyStateViaURLQuery(controls, query); } diff --git a/src/middleware/changeURL.js b/src/middleware/changeURL.js index f34e8af57..deeba892f 100644 --- a/src/middleware/changeURL.js +++ b/src/middleware/changeURL.js @@ -36,6 +36,7 @@ export const changeURLMiddleware = (store) => (next) => (action) => { case types.URL_QUERY_CHANGE_WITH_COMPUTED_STATE: // fallthrough case types.CHANGE_URL_QUERY_BUT_NOT_REDUX_STATE: query = action.query; + if (query.n === 0) delete query.n; if (query.tt) delete query.tt; break; case types.CHANGE_ZOOM: From 2dac45cf46e33f5321cda3dc41ae27a3ad3c6e35 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Thu, 30 Jan 2020 16:34:08 +1300 Subject: [PATCH 5/7] [narrative bugfix] can now start on a page with main-markdown-display --- src/actions/recomputeReduxState.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index bd5e6b3b3..db973180c 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -623,6 +623,11 @@ export const createStateFromQueryOrJSONs = ({ const n = parseInt(query.n, 10) || 0; controls = modifyStateViaURLQuery(controls, queryString.parse(narrative[n].query)); query = n===0 ? {} : {n}; // eslint-disable-line + /* If the narrative block in view defines a `mainDisplayMarkdown` section, we + update `controls.panelsToDisplay` so this is displayed */ + if (narrative[n].mainDisplayMarkdown) { + controls.panelsToDisplay = ["EXPERIMENTAL_MainDisplayMarkdown"]; + } } else { controls = modifyStateViaURLQuery(controls, query); } From 2401e625a534c2248604264fafab124d1e4ba19b Mon Sep 17 00:00:00 2001 From: james hadfield Date: Thu, 30 Jan 2020 16:36:07 +1300 Subject: [PATCH 6/7] [bugfix] allow boolean values for default state mapTriplicate uses a boolean type --- src/actions/recomputeReduxState.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index db973180c..98f248816 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -106,6 +106,7 @@ const modifyStateViaURLQuery = (state, query) => { const restoreQueryableStateToDefaults = (state) => { for (const key of Object.keys(state.defaults)) { switch (typeof state.defaults[key]) { + case "boolean": // fallthrough case "string": { state[key] = state.defaults[key]; break; @@ -115,7 +116,7 @@ const restoreQueryableStateToDefaults = (state) => { break; } default: { - console.error("unknown typeof for default state of ", key); + console.error("unknown typeof for default state of ", key, state.defaults[key]); } } } From db8d538213f3c804084897a8a87d02b3b2f1455d Mon Sep 17 00:00:00 2001 From: james hadfield Date: Thu, 30 Jan 2020 16:43:31 +1300 Subject: [PATCH 7/7] [narrative bugfix] prevent crashes when accessing out-of-bounds pages Could happen if you defined `?n=15` for a narrative with less than 15 pages --- src/actions/recomputeReduxState.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index 98f248816..3607e166b 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -621,7 +621,12 @@ export const createStateFromQueryOrJSONs = ({ URL query this page defines via this information */ if (narrativeBlocks) { narrative = narrativeBlocks; - const n = parseInt(query.n, 10) || 0; + let n = parseInt(query.n, 10) || 0; + /* If the query has defined a block which doesn't exist then default to n=0 */ + if (n >= narrative.length) { + console.warn(`Attempted to go to narrative page ${n} which doesn't exist`); + n=0; + } controls = modifyStateViaURLQuery(controls, queryString.parse(narrative[n].query)); query = n===0 ? {} : {n}; // eslint-disable-line /* If the narrative block in view defines a `mainDisplayMarkdown` section, we