search

juju / juju-gui

Juju-GUI is a web-based GUI for Juju <https://jujucharms.com/>.
Other
182 stars 85 forks source link

Add component analytics collection/sending #3969

Closed huwshimi closed 5 years ago

huwshimi commented 5 years ago

Add new smarter analytics tracker. Fixes: #3965.

webteam-app commented 5 years ago

Starting demo at: https://juju-gui-juju-pr-3969.run.demo.haus/

huwshimi commented 5 years ago

If you are required to addComponent in every call then you might as well just have it as an argument, positional or otherwise, in the sendEvent call.

You only addComponent when you want to add something to the hierarchy, which might not be the component you sendEvent from (e.g. the sendEvent might be in a button component in which you don't necessarily care about).

this.props.analytics.sendEvent(this, analytics.commonEvents.VIEW);

I'm also not keen on having to include a separate lib for the common events, those should be statics on the instance.

Good call.

const analytics = this.props.analytics;
analytics.sendEvent(this, analytics.VIEW);

Having to do this setup everywhere in the component might get tedious if it turns out that it needs to be done in multiple places frequently.

componentDidMount() {
  this.analytics = this.props.analytics.addComponent(this);
}

somemethod() {
  this.analytics.sendEvent(this.analytics.VIEW);
}

Nice. I think it's flexible enough that you can do that where it makes sense.

Then adding to the label while sending the event

  this.analytics.sendEvent(this.analytics.VIEW, 'cs:my-awesome-charm');
  // label value = model committed; user authenticated; cs:my-awesome-charm

I like it, I'll just have to figure out a way to make that flexible enough to work in the case where this is not being used inside a component e.g. on the canvas.

huwshimi commented 5 years ago

QA: