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

Create view for geoprocessing input information #2

Closed
mapsam opened this issue Mar 21, 2015 · 11 comments
Closed

Create view for geoprocessing input information #2

mapsam opened this issue Mar 21, 2015 · 11 comments

Comments

@mapsam
Copy link
Member

mapsam commented Mar 21, 2015

For example, when someone clicks "buffer" they get a screen asking for specific geometries. I'm unsure how to build this one right now. I wonder what the best way to do it is?

@mapsam
Copy link
Member Author

mapsam commented May 3, 2015

screen shot 2015-05-03 at 1 32 14 pm

mapsam added a commit that referenced this issue May 3, 2015
mapsam added a commit that referenced this issue May 3, 2015
@mapsam
Copy link
Member Author

mapsam commented May 4, 2015

dnc-bufferz

@mapsam
Copy link
Member Author

mapsam commented May 4, 2015

Confirming that the second "miles", "km", "meters", etc. are working properly:

dnc-buffertesting

@alukach
Copy link
Member

alukach commented May 4, 2015

This is looking great!

@ranchodeluxe
Copy link
Member

whoohooo!

@mapsam
Copy link
Member Author

mapsam commented May 4, 2015

Thanks!

It's all a bit hacky right now, and I'd like some feedback on the Forms.js as well as my (horrific) attempt to pass events back to the controller and interact with them via subscriptions. I think this event handler is pretty wacky, since I'm looping through the operations to find the Class since all I seem to be able to retrieve is an object copy of the class, that no longer contains the operation.execute() function. Any ideas?

@mapsam
Copy link
Member Author

mapsam commented May 7, 2015

Here's a template for adding a new operation to AppController.js

.addChild(new L.DNC.TurfOperation('buffer', {
    maxFeatures: 1,
    additionalArgs: 0.1,
    description: 'DESCRIPTION.',
    parameters: [

        // PARAMETER 1 OBJECT
        {
            name: 'PARAMETER 1',
            description: 'PARAMETER 1 DESCRIPTION',
            type: 'number',
            default: 10
        },

        // PARAMETER 2 OBJECT
        {
            name: 'PARAMETER 2',
            type: 'select',
            description: 'PARAMETER 2 DESCRIPTION',
            options: ['OPTION 1', 'OPTION 2', 'OPTION 3', 'OPTION 4'],
            default: 'OPTION 3'
        }
    ]
}))

mapsam added a commit that referenced this issue May 7, 2015
mapsam added a commit that referenced this issue May 7, 2015
mapsam added a commit that referenced this issue May 7, 2015
@alukach
Copy link
Member

alukach commented May 7, 2015

The template looks good. I'm starting to worry that putting all this implementation within these lines is making the app difficult to test. I've been looking through some integration test tools lately hoping that one of those could solve this problem.

I think this event handler is pretty wacky, since I'm looping through the operations to find the Class since all I seem to be able to retrieve is an object copy of the class, that no longer contains the operation.execute() function. Any ideas?

Yeah, there seems like there should be an easier way to do that part. The objects should have execute() available, will try to figure out why they don't. Will also look through forms.js when I get a chance.

@mapsam
Copy link
Member Author

mapsam commented May 7, 2015

I wonder if the .addChild() portions are complicating the workflow in AppController.js. Would it be okay to break them apart in to their own pieces for simpler access in the controller? Something like

// create geoprocessing menu with whatever turf operations we want
this.menu.geoprocessing = new L.DNC.MenuBar( /* options */ );

// create new way to access turf operations, which can be run after 
// the event fires with a string like 'buffer'
this.operations.turf = new L.DNC.Operation( /* options, which could be specified in unique files i.e. buffer.js */ );

// now we can execute these operations when an item is clicked
this.menu.geoprocessing.on('clicked', this.operations.turf( /* operation id */ );

@alukach
Copy link
Member

alukach commented May 8, 2015

Yeah, I think you're spot on about the problem relating to addChild(). I don't think I quite understand your example. Does this.menu.geoprocessing define the operation? Or is that within this.operations.turf? And then the actual handling of the operation doesn't occur untilthis.menu.geoprocessing.on`? Is don't quite understand that.

With that said, I think you're right with the idea of breaking out operations into their own files. I wanted to avoid that initially but now I'm thinking that it could improve testability.

When I originally commented, I thought this was a PR. Is this all related to #63?

@mapsam
Copy link
Member Author

mapsam commented May 8, 2015

When I originally commented, I thought this was a PR. Is this all related to #63?

Mostly! This conversation is a result of #63, but maybe not directly related since it involves some refactoring. I'm going to turn that PR into a larger refactor, though. Is that okay with you?

I don't quite understand that.

I'm kinda confusing myself as well. 😜 When you say "breaking out operations into their own files" is what I was imagining when putting that together. They are going to rely on each other, that's for sure, but I think they can all be their own individual Classes without getting spaghetti'd together.

I'm going to play around with some hypothetical ways to break apart the different operations into their own classes and see what happens. I'll share some results later this evening! Going to continue working on #63 and turn it into a larger refactor since I want to keep the Form.js pieces running.

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

No branches or pull requests

3 participants