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

Display and save GeoRSS feeds as map layers #161

Merged
merged 12 commits into from
Jan 3, 2013

Conversation

mbertrand
Copy link
Contributor

FeedSource - generic source for GeoRSS feeds

PicasaFeedSource, YoutubeFeedSource - inherit from FeedSource, with custom feature formats & info balloons for Picasa and Youtube feeds

FeedSourceDialog - form for generating feed layers and adding to map

text: this.addFeedActionMenuText,
handler: this.showFeedDialog,
scope: this
}));
Copy link
Contributor

Choose a reason for hiding this comment

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

Whether the feed item should be added or not should be configurable, like the search and upload items.

…used addOutput for dialog

Updated copyright on new files
Added dependencies
Used filterToParams for PicasaFeedSource and YouTubeFeedSource
Created a separate plugin for feed SelectControl (FeedGetFeatureInfo.js)
Configure Picasa and Youtube popups with XTemplates, use GeoExt.Popup
Modified ptype prefixes
Got rid of 'defaultFormat' and 'format' feedsource properties, replaced with 'format'
Added new text to all language files (but only in English, need to be translated)
Added titles to feed legend styles
Reduced width of FeedSourceDialog from 600 to 400 pixels
Still a couple things left to do
ptype: "gxp_getfeedfeatureinfo",


construction: function(config) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A method named construction will not be executed. But since you're only calling the parent's constructor here, you can remove the whole method.

/** api: ptype = gxp_getfeedfeatureinfo */
ptype: "gxp_getfeedfeatureinfo",

addActions: function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be done in init instead, by registering a listener for the target's "ready" event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something like this?

    init: function(target) {
        gxp.plugins.FeedGetFeatureInfo.superclass.init.apply(this, arguments);
        this.target.on("ready", function() {
            this.target.mapPanel.layers.on({
                "add": this.addLayer,
                "remove": this.removeLayer,
                scope: this
            });
        }, this);
    },

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this looks good.

@@ -248,6 +255,14 @@ gxp.plugins.AddLayers = Ext.extend(gxp.plugins.Tool, {
scope: this
}));
}
if (this.initialConfig.feeds) {
Copy link
Contributor

Choose a reason for hiding this comment

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

feeds config option needs to be documented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added initialConfig.feeds documentation

@ahocevar
Copy link
Contributor

Another general comment: The syntax for the API docs is incorrect in many places. Please build the documentation locally to find issues with the API doc annotations.

@mbertrand
Copy link
Contributor Author

Hi @ahocevar, happy New Year!

Are there any other changes you'd like me to make to this pull request?

@ahocevar
Copy link
Contributor

ahocevar commented Jan 2, 2013

Hey @mbertrand, Happy New Year to you too! I think we're good here. The only reason why I haven't merged yet is because the pull request does not apply cleanly to master. Maybe you can resolve the conflicts?

@mbertrand
Copy link
Contributor Author

Sure @ahocevar, I'll work on that.

On 01/02/2013 10:32 AM, ahocevar wrote:

Hey @mbertrand https://github.com/mbertrand, Happy New Year to you
too! I think we're good here. The only reason why I haven't merged yet
is because the pull request does not apply cleanly to master. Maybe
you can resolve the conflicts?


Reply to this email directly or view it on GitHub
#161 (comment).

Conflicts:
	src/script/locale/ca.js
	src/script/locale/cn.js
	src/script/locale/de.js
	src/script/locale/el.js
	src/script/locale/en.js
	src/script/locale/es.js
	src/script/locale/fr.js
	src/script/locale/id.js
	src/script/locale/nl.js
	src/script/locale/pl.js
	src/script/plugins/AddLayers.js
* Intended to be used for adding the feed
* layer to the map
*/
this.addEvents("addFeed");
Copy link
Contributor

Choose a reason for hiding this comment

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

We use all lowercase event names, so this should be 'addfeed'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to 'addfeed'

On 01/03/2013 08:02 AM, ahocevar wrote:

In src/script/widgets/FeedSourceDialog.js:

  • /**
  • \* api: config[closeAction]
    
  • \* `String` default is destroy
    
  • */
    
  • closeAction: 'destroy',
  • /** private: method[initComponent]
  • */
    
  • initComponent: function() {
  •    /*\* api: event[addFeed]
    
  •     \* Fired after the dialog form is submitted.
    
  •     \* Intended to be used for adding the feed
    
  •     \* layer to the map
    
  •     */
    
  •    this.addEvents("addFeed");
    

We use all lowercase event names, so this should be 'addfeed'.


Reply to this email directly or view it on GitHub
https://github.com/opengeo/gxp/pull/161/files#r2537933.

ahocevar added a commit that referenced this pull request Jan 3, 2013
Display and save GeoRSS feeds as map layers
@ahocevar ahocevar merged commit ba668ce into planetfederal:master Jan 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants