Skip to content
This repository has been archived by the owner on Mar 30, 2021. It is now read-only.

CartoDB Selectors Panels #46

Closed
wants to merge 26 commits into from
Closed

Conversation

shaunanoordin
Copy link
Member

Updated: CartoDB Selector Panels

  • This is a checkpoint PR to ensure we're all in sync and to prevent this branch from growing too big.
  • Added: Selector Panels, which allow users to choose their filter options. Multiple Selector Panels are allowed at a time, each corresponding to a different query, but on the first page load, one default Selector Panel is created.
  • Added: Advanced Mode for Selector Panels, allowing users to type in SQL and CSS queries into CartoDB directly. Powerful, but essentially useless to our intended audience of educators without extensive documentation.
  • Added but hidden (WIP): Guided Mode for Selector Panels, allowing educators to more easily select filters of interest.
  • Added: Custom user input functionality - clicking on a point on the map will inform user of the camera's details.
  • Added: Multiple Map Layers - we have a Satellite view and a Terrain (height/elevation) view, with the option to add as many more as we want.
  • Major Note: There is a notable change in the CartoDB implementation, as our map now uses a Leaflet+CartoDB implementation.
    • Why: We had issues with the CartoDB-only implementation not responding to custom user input (clicking on features)
    • Pros: A base Leaflet layer means we can more easily switch out the CartoDB layer with e.g. a custom-built map visualisation feature (or perhaps Google Maps?)
    • Cons: Our Satellite view layer is worse than the ones we had before, because using Leaflet means accessing 'free' Satellite view 'tile/base layers'. (The CartoDB-only implementation had access to the 'Nokia Satellite Day' map from Here.com, which appears to be paid for in other cases: https://developer.here.com/plans/api/consumer-mapping)

@simoneduca , @rogerhutchings , this PR is ready for review.

"mapVisualisationUrl": "https://shaunanoordin-zooniverse.cartodb.com/api/v2/viz/e04c2e20-a8a9-11e5-8d6b-0e674067d321/viz.json",
"dataTable": "wildcam_gorongosa_cameras_201601",
"dataLayerIndex": 1
"mapExplorer": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reckon we should break this out into a separate json file, it's getting pretty big

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can extract the mapExplorer.species into a separate... let's say... selectorOptions.json file, since filter options like 'species' will only grow as the project evolves, yes, separating it from config.json makes sense. However, at that stage, I wonder if it'd make more sense to extract all Map-related configs and constants (including mapExplorer.cartoDB, mapExplorer.baseLayers, mapExplorer.mapCentre, and the ever-growing list of filter options) into a component-centric logical unit called mapExplorer.config.json. Functionally the same, just a team coding style issue - thoughts?

@rogerhutchings
Copy link
Contributor

So the code itself is fine, but there seems to be a few, monolithic components that do both data and UI. Can you refactor towards a more container / presentational model in a future PR?

Also, it's still using React.createClass rather than ES6 classes

@shaunanoordin
Copy link
Member Author

Sorry about the mess; I'm still exploring the best ways to organise (file & class-wise) the various subcomponents of the Map Explorer. (As the SelectorData data class in SelectorPanel.jsx attests to.)

The React.createClass is a big oversight on my behalf though, so that needs to be fixed immediately. BRB with a new commit.

@shaunanoordin
Copy link
Member Author

Bing! Update. Code cleaned up to remove use of React.createClass, plus various bits of config-and-class-files house keeping.

To future Shaun: when switching from React.createClass to proper ES6 classes, never forget to change...
onChange={this.refreshUI}
to
onChange={this.refreshUI.bind(this)}
because those bindings are no longer implicit.

@shaunanoordin
Copy link
Member Author

Affirmative on that Simone, thanks. Explicit bindings will be moved into the constructor, but on the successor branch I'm working on. Is there anything else for this PR?

@simoneduca
Copy link
Contributor

I'm in the middle of a (long) rebase. I'll finish testing this PR after that and merge if I get no problems, if you're happy with that.

@shaunanoordin
Copy link
Member Author

Sounds good to me! 👍

EXTERNAL DEPENDENCY: Database setup in CartoDB account shaunanoordin-zooniverse
CartoDB query now loads immediately on first loading the Map Explorer.
This improves functionality and addresses bugs where certain input events
wouldn't trigger.
…more.

Next: actually implement selector UI controls.
@shaunanoordin
Copy link
Member Author

Previous issue: Concerns raised that branch was too far behind master.
Action taken: As recommended, branch was rebased to current master
New concerns: Current rebased branch has a lot of deltas on files I haven't personally edited, namely:

  • package.json
  • Index.jsx
  • ActionTypes.js
  • Home.jsx
  • etc

Issue: I am now not sure if this PR is safe to merge Due to the large number of new deltas that I didn't personally create, I can no longer be sure that the stability of master will be maintained
Actions: Investigating alternate options for getting my code into the master

One Option:

  • Create a new branch and new PR with the code I've worked on so far.

@shaunanoordin
Copy link
Member Author

This has been superseded by #49.

@simoneduca simoneduca deleted the cartodb-sql-selectors branch February 23, 2016 14:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants