Skip to content
This repository has been archived by the owner on Apr 17, 2022. It is now read-only.

Key function signature does not seem to match d3 documentation #19

Closed
arnolddevos opened this issue Mar 15, 2016 · 4 comments
Closed

Key function signature does not seem to match d3 documentation #19

arnolddevos opened this issue Mar 15, 2016 · 4 comments

Comments

@arnolddevos
Copy link

The BaseSelection#data method's key argument has type js.ThisFunction2[Datum|NewDatum,js.UndefOr[NewDatum], Int, String]

But the 'this' argument of the key function would be a DOM node or an Array according to d3 documentation here:
https://github.com/mbostock/d3/wiki/Selections#data

Also, the documentation does not mention that the 'd' argument could be undefined.

Perhaps a plain js.Function2[Datum, Int, String] would be better here? It would certainly be easier to use. Supplying a function with the current signature is a pain.

@arnolddevos
Copy link
Author

Rereading the d3 documentation (and testing), it seems undefined can be supplied to the key function. So a signature of js.Function2[js.UndefOr[NewDatum], Int, String] would be needed.

@spaced
Copy link
Owner

spaced commented Mar 16, 2016

Can you give me an example or even better a unit test to describe your expectation? See also here:
https://github.com/spaced/scala-js-d3/blob/master/src/test/scala/org/singlespaced/d3js/SelectionTest.scala

Because there are other fixed issues descibed in #4 and #9. See also my comment #4 (comment)

@arnolddevos
Copy link
Author

Based on all that, my issue boils down to: why Datum|NewDatum in the signature of the key function, s.ThisFunction2[Datum|NewDatum,js.UndefOr[NewDatum], Int, String] ?

I will write some test code and come back.

@arnolddevos
Copy link
Author

Here is a gist to demonstrate the key function arguments: https://gist.github.com/arnolddevos/12972edcfee584333181 (I ran the given code in the browser and collected the log.)

As per d3 documentation, the 'this' argument is either a js.Node or js.Array[Datum]. (In the example Datum=Int.)

So the signature can be js.ThisFunction2[dom.Node|js.Array[Datum], js.UndefOr[NewDatum], Int, String] or js.Function2[js.UndefOr[NewDatum], Int, String] if we don't care about 'this'.

I can do a PR.

edited: type param / can do PR

@spaced spaced closed this as completed in 5d84b4d Mar 31, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants