fillNodes should be an optional step #107

Closed
opened 2015-08-12 00:58:11 -07:00 by ColinEberhardt · 2 comments
ColinEberhardt commented 2015-08-12 00:58:11 -07:00 (Migrated from github.com)

The current public JS API adapts computeLayout to add a fillNodes step, which tramples over any existing layout properties, see #62

I think the public API should make this optional ...

computeLayout(..., false); // fillNodes turned off

Currently within d3fc we do not use the adapted version of the API.

The current public JS API adapts `computeLayout` to add a fillNodes step, which tramples over any existing `layout` properties, see #62 I think the public API should make this optional ... ``` computeLayout(..., false); // fillNodes turned off ``` Currently within d3fc we do not use the adapted version of the API.
vjeux commented 2015-08-12 10:49:03 -07:00 (Migrated from github.com)

I don't like boolean arguments. Can you just make fillNodes exist only in the test environment and not expose it outside?

I don't like boolean arguments. Can you just make fillNodes exist only in the test environment and not expose it outside?
ColinEberhardt commented 2015-08-12 12:10:59 -07:00 (Migrated from github.com)

Hmm ... you're right, thinking about it boolean args lack discoverability and don't self document.

I'll have a rethink.

Hmm ... you're right, thinking about it boolean args lack discoverability and don't self document. I'll have a rethink.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#107
No description provided.