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

Enable fill:none for LineString objects drawn by po.geoJson #111

Open
natevw opened this issue Jan 24, 2012 · 4 comments
Open

Enable fill:none for LineString objects drawn by po.geoJson #111

natevw opened this issue Jan 24, 2012 · 4 comments

Comments

@natevw
Copy link

natevw commented Jan 24, 2012

Right now the geoJson layer draws both LineString and Polygon objects using the SVG element. When dealing with heterogeneous or unknown/generic feature sets (such as one might find in a GeometryCollection) this makes it difficult to style filled polygons without also ending up with "filled" lines, since SVG always applies an implicit "close path" when a fill is specified.

I'd like the mapping from GeoJSON to SVG to work as follows:

  • point → circle
  • linestring → polyline
  • polygon → path
  • multi → g
  • collection → g

Then I could e.g. style all polyline elements in the map with fill:none and not have to deal with special classing/whatnot in a JS stylist workaround.

For my purposes (natevw/Metakaolin#1) I could write a simple enough — non-tiled, non-data-fetching — custom layer to draw geometries in the above fashion, but I'm wondering if this is a reasonable change to submit to trunk on po.geoJson itself?

@mbostock
Copy link
Contributor

What if you set the class attribute on the elements based on the corresponding GeoJSON geometry type? e.g., <circle class="Point">, <path class="LineString">, etc. That way you don't care what elements are used, but you can still style externally.

@natevw
Copy link
Author

natevw commented Jan 25, 2012

That isn't ideal. Essentially your suggestion brings me back to my underlying question: Is the current output of the geoJson layer set in stone (ready for documentation as part of Polymap 2.x's "public" API) or are you open to changes?

If the DOM output is considered stable, then yes, advanced manipulation of the generated SVG is one option, but it seems the least desirable: the features I pass to the GeoJSON layer may have type GeometryCollection, which means in my stylist/load handler I'd need to walk the created DOM elements and match them back to the sub-geometries in the data. This would requires a significant chunk of logic, coupled to the internal decisions the built-in layer makes w.r.t. to its element tree output. So it would not be too much additional work to make a custom, project-specific layer that helps me more cleanly separate the styling concerns from the markup.

If the built-in geoJson layer's DOM is subject to change, though, then I'd like to improve it in the direction outlined above. I think giving each of the three topologies it's own nodeName and always generating groups where the input has nested structure would benefit anyone trying to style non-uniform datasets. (For another example of where the current "turn as much as we can into a single path" makes correct display difficult — combining every polygon within a MultiPolygon into one path means even/odd fill will carve out any overlap between the exterior rings in addition to the expected interior ring holes.)

Hope that clarifies my motivation(s) for this thread?

@mbostock
Copy link
Contributor

Yes, I do consider it part of the public API, but I wouldn't consider it strictly put in stone. It would probably be a major version bump to change the output format of the SVG, because any code currently assuming a given structure (say in external stylesheets, or post-processing) would break.

I'm not sure I understand your objection. I thought you were just asking for a way to set fill: none easily on heterogenous feature collections. Couldn't you do that like so:

.LineString, .MultiLineString {
  fill: none;
  stroke: #000;
  stroke-width: 1.5px;
}

.Point, .MultiPoint, .Polygon, .MultiPolygon {
  fill: #000;
  stroke: none;
}

Is the problem that you want to apply dynamic styles through code to specific features? Or sub-features within a feature or geometry collection? Is the limitation in the po.stylist class or in the DOM structure?

I'm guessing incorporating D3 might be more flexible than the built-in stylist class.

@natevw
Copy link
Author

natevw commented Jan 26, 2012

The problem is these two limitations:

  1. The po.stylist class only facilitates styling at the feature rather than the leaf-geometry level, meaning I'd need to loop through features myself and recursively walk the DOM tree just to annotate the different symbol types. (Theoretically, a GeometryCollection could contain child GeometryCollections!)
  2. The DOM structure for MultiPolygons is wrong, with no real way to workaround the fill issues besides wholesale replacement of the generated SVG in a 'load' event handler. (Could be filed as a separate bug if you'd like.)

I made a comparison between the current and proposed solutions, sorry about the yFrog ads but it's late and it was easiest just to upload the screenshots via Twitter...

(data and source for reference)

Note the two differences: overlapping exterior rings should not make holes, and something like a simple .layer polyline { fill:none; } should be all that's needed to style linear symbols differently than areal. For this demo and Metakaolin's short-term needs, I just made a brand new layer (no xhr/tiling) but the basic ideas could be ported fairly easily into the built-in geoJSON layer.

Basically, by making the SVG output more "semantically meaningful" it becomes easier to style correctly.

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

No branches or pull requests

2 participants