fix(Enrol.Dashboard.Chart.Component) Fix clickHandler

[Jeremy Kells]

clickHandler not passing valid argument to Dashboard

[Brian Lewis]

do you mean in Enrol.District.Chart.Component?

The datum passed to the clickHandler is the datum generating the bar you clicked:

let clickHandler = (datum) => { this.dashboard.options.toggleSelectedDistrict(datum.key); console.log("click", datum);

datum.key should be the districtId for that district. This handler makes that the selected district. It will appear in the options dropdown, and be highlighted in the tables where applicable.

What is the behaviour you are seeing?

What is the value of datum you get in the clickHandler?

[Brian Lewis]

Double-checked the expected behaviour ; the highlighted parts change when clicking a bar in the chart. In particular, the bar itself is highlighted:

[Jeremy Kells]

Really? ok this is odd - I wonder if we have different versions of the chart component

      cht.onClick = (datum) => {
        this.timeout(() => {
          this.dashboard.options.toggleSelectedDistrict(datum.data.key);
          console.log("click", datum);
        }, 0, true);
      };

datum.key is undefined for me - datum.data.key or datum.x works is valid though

[Jeremy Kells]

[Brian Lewis]

I don;t recognize the code sample you posted: it's different to the code I have that I copied above .

this is the relevant part of Enrol.District.Chart.Component:

[Jeremy Kells]

[Brian Lewis]

So when I execute this, with the unchanged code this is the value I have for datum - the expected key-value pair:

and the console shows the expected click events, and onOptionsChanged gets called:

So it may be an issue to do with version of dcjs somehow.

[Jeremy Kells]

Anything in your setup that would have called this override?

[Brian Lewis]

So mystery solved I think, the bower file asks for dcjs 2.1, but there is a 2.2 and I suspect that is how it has resolved on your fresh system. So when I force my version up to 2.2 I see what you see. Sadly there's no documentation to indicate a breaking change.

So can you make a pr from your code please, and could you also please change bower.json:

"dcjs": "^2.2.1",

Hopefully that doesn't raise an other issues ..

[Jeremy Kells]

OK Sounds good. Will do

[Jeremy Kells]

I guess bower doesn't have a -lock file like npm (or yarn). It could be worth moving client side dependency management into npm.

[Brian Lewis]

bower is becoming less used, considered to be deprecated from what I'm reading. But there is a bit more involved in this project because of the way the bower file is scanned to determine what gets deployed and where. You'll see that in the grunt file.

Probably at a good few days work involved I think off the top of my head - what do you think?

typings is another area with similar issues - and npm is also the favoured direction these days.

[Brian Lewis]

chore (Enrol.Dashboard.Chart.Component) Fix ClickHandler on chart

Updated dcjs bower dependancy invoked clickHandler callback with changed parameters, update to fix

Closes #432

→ <<cset a694b700cba8>>

[Jeremy Kells]

At first glance moving the remotables section of the bower file into package.json, and pointing the remotables function in the grunt file at it should work - but the devil is in the details. Probably not enough pain here yet to warrant doing this.

Yes bower is on the way out (slowly, like all legacy solutions!). And then there is yarn, the newer, shinier javascript package manager - probably good for a new build, but not worth migrating to as I believe many of the improvements in yarn have been incorporated into NPM 6.