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

Initial commits of blacklight-maps gem #1

Merged
merged 3 commits into from
Mar 12, 2014
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ test/tmp
test/version_tmp
tmp
spec/internal
jetty
jetty
.DS_Store
11 changes: 5 additions & 6 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
source 'https://rubygems.org'

gem 'leaflet-rails'

gem 'leaflet-markercluster-rails', git: 'https://github.com/mejackreed/leaflet-markercluster-rails.git'

gem 'leaflet-sidebar-rails', '0.0.0', git: 'https://github.com/mejackreed/leaflet-sidebar-rails.git'

# Specify your gem's dependencies in blacklight-maps.gemspec
gemspec

gem 'simplecov', require: false
gem 'coveralls', require: false

gem 'engine_cart', git: 'https://github.com/cbeer/engine_cart'
Copy link
Member

Choose a reason for hiding this comment

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

I suspect this gem engine_cart can get dropped. The gemspec should be providing it, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cbeer There was some reason I left it in there and now I can't remember. I'll look into it.



file = File.expand_path("Gemfile", ENV['ENGINE_CART_DESTINATION'] || ENV['RAILS_ROOT'] || File.expand_path("../spec/internal", __FILE__))
if File.exists?(file)
puts "Loading #{file} ..." if $DEBUG # `ruby -d` or `bundle -v`
Expand Down
56 changes: 54 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
# Blacklight::Maps

TODO: Write a gem description
[![Build Status](https://travis-ci.org/sul-dlss/blacklight-maps.png?branch=master)](https://travis-ci.org/sul-dlss/blacklight-maps)

Provides a map view for Blacklight search results.

![Screen shot](docs/map-view.png)
![Screen shot](docs/map-sidebar.png)

## Installation

Expand All @@ -18,7 +23,54 @@ Or install it yourself as:

## Usage

TODO: Write usage instructions here
Blacklight-Maps adds a map view capability for a results set that contains geospatial coordinates (latitude/longitude).

For now, Blacklight-Maps requires that your SOLR index includes a field containing placenames with latitude and longitude coordinates delimited by `|`. This field can be multivalued.

A document requires the following field:
```
placename_coords:
- China|35.86166|104.195397
- Tibet|29.646923|91.117212
- India|20.593684|78.96288
```

Note: We are looking at implementing support for additional fields.

### Configuration

#### Required
Blacklight-Maps expects you to provide:

- a field to map the placename coordinates (`placename_coords` in the example above)

#### Optional

- the maxZoom [property of the map](http://leafletjs.com/reference.html#map-maxzoom)
- a [tileLayer url](http://leafletjs.com/reference.html#tilelayer-l.tilelayer) to change the basemap
- an [attribution string](http://leafletjs.com/reference.html#tilelayer-attribution) to describe the basemap layer


All of these options can easily be configured in `CatalogController.rb` in the `config` block.

```
...
configure_blacklight do |config|
## Default parameters to send to solr for all search-like requests. See also SolrHelper#solr_search_params
config.default_solr_params = {
:qt => 'search',
:rows => 10,
:fl => '*'
}

## Default values
config.view.maps.placename_coords_field = "placename_coords"
config.view.maps.tileurl = "http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"
config.view.maps.attribution = 'Map data &copy; <a href="http://openstreetmap.org">OpenStreetMap</a> contributors, <a href="http://creativecommons.org/licenses/by-sa/2.0/">CC-BY-SA</a>'
...

```


## Contributing

Expand Down
2 changes: 1 addition & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,4 @@ namespace :blacklight_maps do
cp("#{f}", 'jetty/solr/blacklight-core/conf/', :verbose => true)
end
end
end
end
7 changes: 6 additions & 1 deletion app/assets/javascripts/blacklight-maps.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// for Blacklight.onLoad:
//= require blacklight/core

//= require leaflet
//= require leaflet.markercluster
//= require L.Control.Sidebar
//= require L.Control.Sidebar

//= require_tree './blacklight-maps'
137 changes: 137 additions & 0 deletions app/assets/javascripts/blacklight-maps/blacklight-maps-browse.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
var map, sidebar;

Blacklight.onLoad(function() {

// Stop doing stuff if the map div isn't there
Copy link
Member

Choose a reason for hiding this comment

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

@jkeck you mentioned using data-* attributes instead of an ID (which, among other things, could allow multiple maps on the same page, or lend itself to reuse in other places). I think we could do something like

$("[data-blacklight-map]").blacklight_leaflet_map();

$.fn.blacklight_leaflet_map = function(arg_opts) {               
  this.each(function() {
    L.map(this.id)...
  }
}

if ($("#blacklight-map").length === 0){
return;
}

// Get the configuration options from the data-attributes
$.extend(Blacklight.mapOptions, $("#blacklight-map").data());

map = L.map('blacklight-map').setView([0,0], 2);
L.tileLayer(Blacklight.mapOptions.tileurl, {
Copy link
Member

Choose a reason for hiding this comment

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

To make this more extensible, I think it might help to split each of these components out into its own method.

attribution: Blacklight.mapOptions.mapattribution,
maxZoom: Blacklight.mapOptions.maxzoom
}).addTo(map);

// Sets up leaflet-sidebar
sidebar = L.control.sidebar('blacklight-map-sidebar', {
Copy link
Member

Choose a reason for hiding this comment

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

If we're going to support reuse, we should derive the sidebar id from the container.

position: 'right',
autoPan: false
});

// Adds leaflet-sidebar control to map (object)
map.addControl(sidebar);

// Create a marker cluster object and set options
markers = new L.MarkerClusterGroup({
showCoverageOnHover: false,
spiderfyOnMaxZoom: false,
singleMarkerMode: true,
animateAddingMarkers: true
});

geoJsonLayer = L.geoJson(geojson_docs, {
onEachFeature: function(feature, layer){
layer.defaultOptions.title = feature.properties.placename;
layer.on('click', function(e){
if (sidebar.isVisible()){
Copy link
Member

Choose a reason for hiding this comment

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

Extracting the nested click handler would be nice.

Copy link
Member

Choose a reason for hiding this comment

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

and it seems like it's pretty similar to the clusterclick event handler. Maybe we can reuse the code there?

sidebar.hide();
}
var placenames = {};
placenames[feature.properties.placename] = [feature.properties.html];
offsetMap(e);
$('#blacklight-map-sidebar').html(buildList(placenames));
sidebar.show();
});
}
});

// Add GeoJSON layer to marker cluster object
markers.addLayer(geoJsonLayer);

// Add marker cluster object to map
map.addLayer(markers);

// Listeners for marker cluster clicks
markers.on('clusterclick', function(e){

//hide sidebar if it is visible
if (sidebar.isVisible()){
sidebar.hide();
}

//if map is at the lowest zoom level
if (map.getZoom() === Blacklight.mapOptions.maxzoom){

var placenames = generatePlacenamesObject(e.layer._markers);


offsetMap(e);

//Update sidebar div with new html
$('#blacklight-map-sidebar').html(buildList(placenames));

//Show the sidebar!
sidebar.show();
}
});

//Add click listener to map
map.on('click', function(e){

//hide the sidebar if it is visible
if (sidebar.isVisible()){
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should wrap this logic up in a function, so we could call, e.g.:

map.on('click,drag', hideSidebar)

(and I think that's the jquery syntax for matching multiple events. might be wrong..)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll look into this. Looks like this is supported in Leaflet too: http://stackoverflow.com/questions/14677974/single-event-handler-for-multiple-events-in-leaflet

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I didn't realize this wasn't the normal jquery on handler. If we can collapse it, that's great. I think a new method is probably warranted anyway (we call these same lines above too)

sidebar.hide();
}
});

//drag listener on map
map.on('drag', function(e){

//hide the sidebar if it is visible
if (sidebar.isVisible()){
sidebar.hide();
}
});

});

Blacklight.mapOptions = {
tileurl : 'http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png',
mapattribution : 'Map data &copy; <a href="http://openstreetmap.org">OpenStreetMap</a> contributors, <a href="http://creativecommons.org/licenses/by-sa/2.0/">CC-BY-SA</a>'
};

function buildList(placenames){
var html = "";
$.each(placenames, function(i,val){
html += "<h2>" + i + "</h2>";
html += "<ul class='sidebar-list'>";
$.each(val, function(j, val2){
html += val2;
});
html += "</ul>";
});
return html;
}

// Generates placenames object
function generatePlacenamesObject(markers){
var placenames = {};
$.each(markers, function(i,val){
if (!(val.feature.properties.placename in placenames)){
placenames[val.feature.properties.placename] = [];
}
placenames[val.feature.properties.placename].push(val.feature.properties.html);
});
return placenames;
}

// Move the map so that it centers the clicked cluster TODO account for various size screens
function offsetMap(e){
mapWidth = $('#blacklight-map').width();
mapHeight = $('#blacklight-map').height();
map.panBy([(e.originalEvent.layerX - (mapWidth/4)), (e.originalEvent.layerY - (mapHeight/2))]);
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
/* Master manifest file for engine, so local app can require
* this one file, but get all our files -- and local app
* require does not need to change if we change file list.
*
*= require 'leaflet.markercluster'
*= require 'leaflet.markercluster.default'
*= require 'L.Control.Sidebar'
*/
23 changes: 20 additions & 3 deletions app/assets/stylesheets/blacklight_maps/default.css.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,24 @@
#documents.map {
/*
*= require leaflet
*= require leaflet.markercluster
*= require leaflet.markercluster.default
*= require L.Control.Sidebar.css
*/

.view-icon-maps {
&:before { content: "\e135"; }
}

.view-icon-maps {
&:before { content: "\e062"; }
#blacklight-map{
height: 550px;
}

.sidebar-thumb{
height: 64px;
width: 64px;
}

.sidebar-list{
padding-left: 0;
list-style: none;
}
36 changes: 36 additions & 0 deletions app/helpers/blacklight_maps_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
module BlacklightMapsHelper

def show_map_div
Copy link
Member

Choose a reason for hiding this comment

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

To give our consumers additional flexibility, I'd adjust this method signature to something like:

def blacklight_map_tag id, tag_options = {}, &block
  ...
  content_tag(:div, { id: id  data: ...}.deep_merge(tag_options), &block)
end

This (sorta) mirrors the regular #content_tag signature, and allows the caller to add additional attributes and the like to this div.

data_attributes = {
:maxzoom => blacklight_config.view.maps.maxzoom,
:tileurl => blacklight_config.view.maps.tileurl

}

content_tag(:div, "", id: "blacklight-map",
data: data_attributes
)
end

def serialize_geojson
geojson_docs = {type: "FeatureCollection", features: []}
@response.docs.each_with_index do |doc, counter|
Copy link
Member

Choose a reason for hiding this comment

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

I don't see where we're using the index counter. Maybe it's vestigial?

We can also use #map to create the features array directly, e.g.:

{type: "FeatureCollection", features: @response.docs.map { |doc| ... }.compact }.to_json

Copy link
Member

Choose a reason for hiding this comment

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

Instead of rendering @response.docs directly, I'd add (at least as an option) the ability to pass an array of documents in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cbeer +1 on index counter.

if doc[blacklight_config.view.maps.placename_coord_field]
Copy link
Member

Choose a reason for hiding this comment

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

I think we could push some of this logic into the document, using the "Document Extensibility Framework" to add an #export_as_geojson.

doc[blacklight_config.view.maps.placename_coord_field].each do |loc|
values = loc.split('|')
Copy link
Member

Choose a reason for hiding this comment

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

We should wrap the loc parsing in a separate helper or model method, especially if it'd let us write

doc.geofeatures.each &block

feature = {type: "Feature", geometry: {type: "Point",
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, but does leaflet support other geometries too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cbeer yes it does, -pretty much everything that is out there. My thought was initially supporting this format "placename-coord" (kind of its own solr thing) and expanding to polygons (bounding boxes) and coordinates without the placenames. The Spotlight specific use case relies on these coordinates relating to placenames.

Copy link
Member

Choose a reason for hiding this comment

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

Cool. I can imagine for e.g. maps of africa, being able to show bounding boxes could be pretty interesting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, BPL wants this too.

coordinates: [values[2].to_f, values[1].to_f]},
properties: {placename: values[0],
html: render_leaflet_sidebar_partial(doc)}}
Copy link
Member

Choose a reason for hiding this comment

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

I think/hope the render_leaflet_sidebar_partial call can be replaced with something like:

render_to_string(doc) # not sure if render_to_string supports this short-hand.. but it'd be awfully nice if it did.

Obviously, if we delegate most of the geojson to the model, we'll have to have some way to pass or merge the html into the block..

geojson_docs[:features].push feature
end
end
end
return geojson_docs.to_json
end

def render_leaflet_sidebar_partial(doc)
render partial: 'catalog/index_maps', locals: {document: SolrDocument.new(doc)}
end

end
6 changes: 4 additions & 2 deletions app/views/catalog/_document_maps.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
<% # container for all documents in index view -%>
<% # container for all documents in map view -%>
<div id="documents" class="map">
<!-- map goes here -->
<%= show_map_div() %>
<div id="blacklight-map-sidebar"></div>
Copy link
Member

Choose a reason for hiding this comment

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

I didn't realize above that the #blacklight-map-sidebar was created here. Can we maybe push this into show_map_div? And, maybe, add a data-* attribute to connect the map with the sidebar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cbeer are you thinking something like data-mapsidebar=1?

Copy link
Member

Choose a reason for hiding this comment

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

Something like:

<div id="blacklight-maps" data-map-sidebar="#blacklight-map-sidebar" />

And then we can use that attribute in the javascript to figure out where the sidebar is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cbeer ok, just checked this with leaflet. It will not work with a data-attribute but only seems to work with an id. Same with the sidebar.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I didn't assume leaflet would do that work. In our wrapping javascript, we'd have to parse it out and use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh got it.

<%= javascript_tag "var geojson_docs = #{serialize_geojson}" %>
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using global variables, can we turn this into a method call? Maybe something like:

$("#blacklight-map").blacklight_leaflet_map(<%= serialize_geojson %>);

</div>
9 changes: 9 additions & 0 deletions app/views/catalog/_index_maps.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<% # the way each document will be viewed in the sidebar list -%>
<li class='media'>
<%= render_thumbnail_tag document, {class: 'sidebar-thumb media-object'}, {class: 'pull-left'} %>
<div class='media-body'>
<h4 class='media-heading'>
<%= link_to_document document, :label=>document_show_link_field(document) %>
</h4>
</div>
</li>
3 changes: 3 additions & 0 deletions blacklight-maps.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ Gem::Specification.new do |spec|
spec.add_dependency "rails"
spec.add_dependency "blacklight", ">= 5.1.0"
spec.add_dependency "bootstrap-sass", "~> 3.0"
spec.add_dependency "leaflet-rails"
spec.add_dependency "leaflet-markercluster-rails"
spec.add_dependency "leaflet-sidebar-rails", "~> 0.0.2"

spec.add_development_dependency "bundler", "~> 1.5"
spec.add_development_dependency "rake"
Expand Down
Binary file added docs/map-sidebar.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/map-view.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 15 additions & 1 deletion lib/blacklight/maps/engine.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
require 'blacklight'
require 'leaflet-rails'
require 'leaflet-markercluster-rails'
require 'leaflet-sidebar-rails'


module Blacklight
module Maps
class Engine < Rails::Engine
Blacklight::Configuration.default_values[:view].maps.lat_lng_field = "zzz_pt"

# Set some default configurations
Blacklight::Configuration.default_values[:view].maps.placename_coord_field = "placename_coords"
Blacklight::Configuration.default_values[:view].maps.tileurl = "http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"
Blacklight::Configuration.default_values[:view].maps.mapattribution = 'Map data &copy; <a href="http://openstreetmap.org">OpenStreetMap</a> contributors, <a href="http://creativecommons.org/licenses/by-sa/2.0/">CC-BY-SA</a>'
Blacklight::Configuration.default_values[:view].maps.maxzoom = 8

# Add our helpers
initializer 'blacklight-maps.helpers' do |app|
ActionView::Base.send :include, BlacklightMapsHelper
end

# This makes our rake tasks visible.
rake_tasks do
Expand Down
Loading