NUKnightLab / storyline

Tell the story behind the numbers.
https://storyline.knightlab.com
Mozilla Public License 2.0
124 stars 21 forks source link

fix: Updating config builder so config values are forced to be string #76

Open shortdiv opened 7 years ago

shortdiv commented 7 years ago

Now the value start_at_card can be either string or num regardless of whether cards are read in from spreadsheet or local json. Fixes #61

{
  "data": {...},
  "chart": {...},
  "slider": {
    "title_column_name": "slide title",
    "text_column_name":  "slide text",
    "start_at_card": 0 // works for explicit num e.g. `0` OR column name from spreadsheet e.g. `"slide active"`
  }
}

Not a fan of the mixed approach bc it means additional checks to ensure that the value gets propagated. I think it'd be a better approach if we were explicit in the docs about how to create these config values— either use spreadsheet columns for all card config vals OR use locally declared strings via json.

shortdiv commented 7 years ago

@JoeGermuska thoughts? ^

JoeGermuska commented 7 years ago

If we want to have a not-mixed approach, I'd advocate for using a zero-based integer value in slider.start_at_card and disregarding spreadsheet configuration.

I think using the spreadsheet to define the 'start' card is weird.