Problem/Motivation

Since 7.x-1.0-alpha5 release of Views GeoJSON it's advised to use feed displays instead of page for better performance.

There are a warning at display settings window "Using the GeoJSON display style on "Page" displays is deprecated, but will continue to work until the next release.

hook_leaflet_geojson_source_info() currently is hardcoded to look for page displays. Which won't work anymore for the next Views GeoJSON release.

Proposed resolution

Don't check the display type, instead use the views API function $view->display_handler->get_url().
If it exposes an url and has the style views_geojson we should be set to use the view.

Remaining tasks

Reviews needed.

User interface changes

None.

API changes

None.

Original report by @okolobaxa

Since 7.x-1.0-alpha5 release of Views GeoJSON advised to use feed displays instead of page for better performance.

There are a warning at display settings window "Using the GeoJSON display style on "Page" displays is deprecated, but will continue to work until the next release. Instead, the GeoJSON display style should be used with a "Feed" display."

But now hook_leaflet_geojson_source_info() of Leaflet GeoJSON module is searched only for page displays.

CommentFileSizeAuthor
#7 leaflet_geojson-more-robust-views-detection-2367459-7.patch1.63 KBdas-peter
#2 leaflet_geojson-more-robust-views-detection-2367459-2.patch1.59 KBdas-peter
#1 search_feed_display-2367459-1.patch856 bytesokolobaxa

Comments

okolobaxa’s picture

okolobaxa commented
Assigned: Unassigned » okolobaxa
Status: Active » Needs review
StatusFileSize
new search_feed_display-2367459-1.patch856 bytes

Here is small patch for this problem. Tested on 2.x branch, but i think works same on 1.x

das-peter’s picture

das-peter commented
Issue summary: View changes
StatusFileSize
new leaflet_geojson-more-robust-views-detection-2367459-2.patch1.59 KB

I think we should tackle the issue by a more robust solution.
How about checking if the view actually exposes an url by using the API function $view->display_handler->get_url().
If it exposes an url and has the style views_geojson we should be all set to use the view.
Further the code to detect if there's a bbox argument should be more defensive to avoid notices.

mpgeek’s picture

mpgeek commented
Status: Needs review » Needs work

I would agree with @das-peter here. His concept is more generic. @okolobaxa maybe you could generalize your patch. If it doesn't make sense to generalize, please do explain.

das-peter’s picture

das-peter commented
Status: Needs work » Needs review

@mpgeek Was the status change intentionally? If so, what needs work? Anything that should changed in the patch? If not (yet) - I guess this still just needs reviews. And everyone is welcome to point things out that need actual work :)

mpgeek’s picture

mpgeek commented

@das-peter, your status is correct. Lets get this reviewed.

okolobaxa’s picture

okolobaxa commented

Thanks for you answers! I agree, patch by @das-peter in better solution, let's use it.

das-peter’s picture

das-peter commented
StatusFileSize
new leaflet_geojson-more-robust-views-detection-2367459-7.patch1.63 KB

Just figured out we should check for the old and new style plugin:
in_array($style_plugin, array('views_geojson_feed', 'views_geojson')).
Patch updated.

  • 329c95d committed on 7.x-2.x 
    Issue #2367459 by das-peter, okolobaxa, dasjo: Fixed Search for feed...
dasjo’s picture

dasjo
he/him
Primary language German
Location Zurich
commented
Status: Needs review » Fixed

Thanks!

I had to reroll the patch one onto the current dev version, committed :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.