A few suggestions for the tree layout #843
Conversation
| function pathJoin(p) { | ||
| return Array.isArray(p) ? p.map(str => `${str}`.replace(/\//g, '\\/')).join("/") : p; | ||
| } |
There was a problem hiding this comment.
I don’t think we should accept arrays. I think limiting the path to strings is simpler.
| @@ -217,7 +221,7 @@ function nodePath(node) { | |||
| } | |||
|
|
|||
| function nodeName(node) { | |||
| return node.id.split("/").pop(); | |||
| return node.id.replace(/\\\//g, "\0").split("/").pop().replace(/\0/g, "/"); | |||
There was a problem hiding this comment.
This makes me realize that we should handle escaped backslashes like we do in d3-hierarchy:
We shouldn’t use \0 and hope that it doesn’t exist in the path. 😄
| @@ -4,7 +4,7 @@ import {channel, isObject, one, valueof} from "../options.js"; | |||
| import {basic} from "./basic.js"; | |||
|
|
|||
| export function treeNode({ | |||
| path, // the delimited path | |||
| path = d => d, // the delimited path | |||
There was a problem hiding this comment.
We should use identity from ../options.js here.
| m.plot = function({x = {axis: null}, y = {axis: null}, inset = undefined, insetLeft = inset != null ? inset : 10, insetTop = 20, insetBottom = 20, insetRight = 120, ...options} = {}) { | ||
| return plot.call(this, {x, y, insetLeft, insetRight, insetTop, insetBottom, ...options}); | ||
| }; |
There was a problem hiding this comment.
I thought about this, too, but I don’t think the mark.plot shorthand should have different defaults from Plot.plot. If we want these hints we should figure out how to do them as channel hints.
| @@ -38,7 +38,7 @@ export function treeNode({ | |||
| for (const o of outputs) o[output_values] = o[output_setValues]([]); | |||
| for (const facet of facets) { | |||
| const treeFacet = []; | |||
| const root = rootof(facet).each(node => node.data = data[node.data]); | |||
| const root = rootof(facet.filter(i => P[i] != null)).each(node => node.data = data[node.data]); | |||
Not added, but I added string coercion and you can use a comma as a delimiter to approximate this. (But that kind of defeats the reason to use an array, which is that you don’t have to escape separators, so I’d still recommend escaping separators and passing strings.)
Added.
Added, using Plot.identity.
Not added; I think these need to be channel hints rather than _mark.plot shorthand, but I don‘t feel like it’s worth the trouble to implement these yet.
Added. |
TODO: