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

Raise configuration up #87

Open
rod-glover opened this issue Apr 2, 2020 · 3 comments
Open

Raise configuration up #87

rod-glover opened this issue Apr 2, 2020 · 3 comments
Labels
Milestone

Comments

@rod-glover
Copy link
Contributor

Too many components (and perhaps some data services) know about and draw directly on configuration information.

At present we have a fairly flat render hierarchy, somewhere around 3 levels:

  • Component App builds much of the app out of the much lower level components.
  • One or two mid-level such as TwoDataMaps which deliver a high-level view composed in several layers (in this case, much of the Maps tab content).
  • Many lower level components (e.g., selectors, tabbing, text content).

Many of the lower level components get configuration information for themselves. Instead, this information should be obtained only by components at the highest levels of the hierarchy and provided as props to the lower level components.

This will have the following benefits:

  • Lower components are more easily reused, refactored, or repurposed.
  • Easier to find where configuration information is extracted and used in the app.

The disadvantage of this change will be more props being passed around.

@rod-glover rod-glover added this to the Code cleanup milestone May 22, 2020
@jameshiebert
Copy link

jameshiebert commented Jun 2, 2020

Not sure if this is a separate issue, or if it's a sub-bullet of your comment above...

It would be useful if the selections (region, futureTimePeriod, season, variable), could be raised up so that they be set in the URL somehow. E.g. our current landing page lets the user configure the options before even hitting the app, and it's useful for agencies that are writing reports and need to link to their source analysis.

It's not a high priority, but something to keep in mind as we move forward.

@rod-glover
Copy link
Contributor Author

rod-glover commented Jun 2, 2020

I'm glad to hear you suggest this: it's been on my mind for a couple of months now. However it is different than configuration (which is more or less static), so I have created a new issue for it.

@rod-glover
Copy link
Contributor Author

rod-glover commented Jul 2, 2020

Additionally: There are a lot of (more or less) app-wide configurations -- those not specific to a tab or other sub-area -- that are retrieved separately (e.g., variables, units, seasons). It would make sense to refactor the config file to place these under a single app-wide key at the top level of the config file. This app-wide config can be passed into every individual item that needs it.

And, given that at least one item (units) in the config file undergoes post-processing before usage in the app, it would make sense to do retrieval and processing only once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants