Skip to content
This repository has been archived by the owner on Dec 1, 2020. It is now read-only.

Move core services into their own module #137

Merged
merged 4 commits into from
Nov 7, 2015
Merged

Move core services into their own module #137

merged 4 commits into from
Nov 7, 2015

Conversation

tomwayson
Copy link
Member

Created esri.core module as a dependency of esri.map.
Added a test page to demonstrate using the core module to define your own directive.

new folder structure
- core
- layers
- map

core includes:
 - esriLoader
 - esriRegistry
 - esriMapUtils
 - esriLayerUtils

moved instance-specific code from esriLayerUtils into a base controller
other layer controllers extend this base controller for shared functions
build core module into it's own file
based on the blend renderer sample
@tomwayson
Copy link
Member Author

Resolves #69

@jwasil please review, specifically:

  • Did I get the breakdown right in terms of what is/isn't in core?
  • Layer controllers extending a base controller - I'm not super stoked on this, but it's working. Kinda goes back to what @mpriour suggested a long time ago - should just be one layer directive w/ a type attribute ("feature", "dynamic", etc). Extending a base controller seems like a compromise that let's us keep our current directive API
  • Folder structure, file naming, etc. This should be the final API for 1.0, so let's make sure it's right.
  • I don't think the sample I used for the custom directive test page makes for the best demo of a custom directive (doesn't have any bound attributes).I wanted to use the raster slider example, but I didn't want to deal w/ getting an angular slider (this one seems like the best non-UI framework one) working w/ it (it was late). I'm open to suggestions for other samples, or one of us could give the raster slider a go, but probably after merging and under a new issue.

@tomwayson tomwayson mentioned this pull request Nov 5, 2015
@tomwayson tomwayson changed the title Breaks core services out into their own module Move core services into their own module Nov 5, 2015
…ome binding for custom directive example; layerInfo.hideLayers number conversion
@jwasilgeo
Copy link
Contributor

Wonderful work here! Great updates through and through.

  • Breakdown of core and non-core: yeah!
  • Shared base layer controller: I modified the bindLayerEvents method a bit so that non-shared event bindings could move out of link and up into the layer-specific controller files.
  • Folder structure: I know we're not really in the business of providing dijit wrappers in this repo, but should the esriLegend be moved to a new folder other than "map"?
  • New example for custom directive: nice! BlendRenderer is cool to see. I added some bindings for basemap and layer visibility. And yes raster slider would be great--let's plan on doing that in another issue--would be a good showcase of JSAPI functionality regardless of framework.
  • Housekeeping for TODO comments in src that we haven't explicitly discussed in awhile:
    • I think layerInfo is fine according to JSAPI docs, but I just added a Number conversion to hideLayers.
    • esriMapUtils.createWebMap: I agree, not having to pass mapController would be nice, but seems tricky to solve right now.
    • TimeExtent and other nice-to-haves for dynamic layer in esriLayerUtils: Yeah, for this PR seems totally fine; do we want to put in issues for another RC later down the road?

Thoughts on above items or what was pushed in 217e818, @tomwayson?

@jwasilgeo jwasilgeo mentioned this pull request Nov 7, 2015
tomwayson added a commit that referenced this pull request Nov 7, 2015
Move core services into their own module
@tomwayson tomwayson merged commit 1706d47 into master Nov 7, 2015
@tomwayson tomwayson deleted the modules branch November 7, 2015 23:07
@tomwayson
Copy link
Member Author

@jwasil thanks for the thorough review. I like your additions and have merged them (after a few minor changes to names and wording).

Thoughts:

  • we'll leave out the "nice to haves" from the API at this point. Just need to remember to document the unsupported dynamic map service layer options when writing docs.
  • I had the same thought about a folder for esriLegend. I don't even know what I would call the folder for esri/dijit type directives. Sure as hell not "dijit." Let's leave it under map for now.

The v.1 API is now complete! Let's add some tests, docs, and put a bow on this thing.

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.

2 participants