search

chartwerk / core

GNU General Public License v3.0
0 stars 0 forks source link

options first in pod constructor #10

Open jonyrock opened 3 years ago

jonyrock commented 3 years ago

I believe that options should be first in the constructor here

abstract class ChartwerkPod<T extends TimeSerie, O extends Options> {
  // ...

  constructor(
    // maybe it's not the best idea
    _d3: typeof d3,
    protected readonly el: HTMLElement,
    protected readonly series: T[] = [],
    _options: O
  ) {
jonyrock commented 3 years ago

@rozetko please say what do you think

rozetko commented 3 years ago

Not sure, because a pod can't render anything without el and series but can without options. I'd just make it optional (don't now why it's not)