ParasolJS / parasol-es

ES6 module for interactive visualization of multi-objective optimization problems
https://parasoljs.github.io/
MIT License
26 stars 5 forks source link

ps.charts.forEach() syntax is confusing #7

Closed wraseman closed 5 years ago

wraseman commented 6 years ago

Going through the examples, I found the following syntax to be very common.

  // create Parasol object
  var ps = Parasol(data)('.parcoords');

  // update charts
  ps.charts.forEach(
    (pc) => {
      pc
        .reorderable()
        .render()
        .updateAxes(0)
    }
  )

I think that this would be more intuitive to add a function to the API that does this. For instance:

  // create Parasol object and render with default settings
  var ps = Parasol(data)('.parcoords')
    .visualize();  // or whatever name makes sense

I figure that way, advanced users can still use the (pc) => form if they want to apply Parcoords options globally but other users won't have to worry about it.

joshhjacobson commented 6 years ago

It seems like what you're suggesting would be a wrapper for the entire Parcoords library such that the user could apply a set of methods globally (to all charts)? For example,

ps.wrapper(reorderable=true, color='blue', etc... )

Users could still edit them individually with something like

ps.charts[i].reorderable()
wraseman commented 6 years ago

Yes, I agree that I'm looking for the user to be able to apply Parcoords methods globally (to all charts). Why not just use the same exact syntax as Parcoords but when applied to a Parasol object it would apply it globally?

ps.reorderable()
  .render()

Under the hood it would do this...

  ps.charts.forEach(
    (pc) => {
      pc
        .reorderable()
        .render()
    }
  )

And then edit individual charts as you described above.

wraseman commented 6 years ago

Update from 10/17 meeting: ps.reorderable(), for example, should also take an argument chartIDs which is an array which charts should be affected. The default for chartIDs will be all.

joshhjacobson commented 6 years ago

I don't think we should try to tackle every feature from parcoords. I'll send a list of the important ones that I plan to start with.

joshhjacobson commented 5 years ago

In the process of writing the first of these wrapper functions, I've begun to question whether we should include chartIDs as an argument. When included, I think there is too much overlap with the functionality of parcoords -- which already does the job very well. Moreover, the arguments need to be passed as an object in order to allow the default behavior to affect all charts but also have the option to specify chartIDs, a detail which makes the function noticeably different and more complicated than the parcoords counterpart. Here is an example:

ps.scale({chartIDs: [1], axis: 'year', domain: [30, 90]});

Instead, I propose that we return to the initial idea of just making the function call apply to all charts. This way is less ambiguous and could replicate the parcoords syntax. For example:

ps.scale('year', [30, 90]);

In general, I think the global effect is what most users will be looking for anyway. Of course, if they do need to edit individual charts, they can still use parcoords directly:

ps.charts[id].anyParcoordsMethod();

@wraseman what are your thoughts on this?

wraseman commented 5 years ago

I really like that idea! Go ahead with that.

joshhjacobson commented 5 years ago

Sounds great!

joshhjacobson commented 5 years ago

A list of implemented wrappers can be found in the wrap directory. Unfortunately, ps.dimensions() and ps.bundleDimension() are broken. I'll open a separate issue for those when I have a better idea of what's going on.