Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigation between dashboard pages #23

Closed
curran opened this issue Sep 14, 2016 · 6 comments · Fixed by #149
Closed

Navigation between dashboard pages #23

curran opened this issue Sep 14, 2016 · 6 comments · Fixed by #149
Assignees
Milestone

Comments

@curran
Copy link
Contributor

curran commented Sep 14, 2016

Add navigation functionality such that clicking on a bar (which represents an app) in the overview dashboard navigates to the app-specific dashboard (parameterized by the clicked app).

Depends on #22, #21, and #20

@curran
Copy link
Contributor Author

curran commented Oct 3, 2016

I wonder what this will look like in the configuration. Our first use case is that clicking on a bar uses the value for that bar as the value for a query parameter.

Currently, adding the onBarClick handler get the original row objects from the tabified data. For example, after listening for the bar click like this

barChart.onBarClick((d) => console.log(JSON.stringify(d, null, 2)));

Clicking on each of the 3 bars prints the following:

{
  "key": "myApp-0",
  "doc_count": 23744,
  "Sum of MB": 86.69855308532715
}
{
  "key": "myApp-1",
  "doc_count": 21277,
  "Sum of MB": 71.80753326416016
}
{
  "key": "myApp-2",
  "doc_count": 4448,
  "Sum of MB": 15.981668472290039
}

We ultimately want to take the value of key and use that as a URL parameter. For example, clicking on the first bar should take you to

http://localhost:3000/dashboards/appDetails?app=myApp-0

Therefore the configuration should be able to specify an onBarClick handler in terms of the source key key and the destination param app.

Here's a very rough sketch of what it might look like:

"navigation": {
  "event": "onBarClick",
  "destinationRoute": "/dashboards/appDetails",
  "sourceKey": "key"
  "destinationParam": "app"
}

@t00f Any thoughts on this?

@t00f
Copy link
Contributor

t00f commented Oct 3, 2016

It looks like a great idea to store these information in the configuration file !

We have two kinds of event:

A) Those which will change the route completely
B) Those which will add new parameters in the query part of the current route.

I have a few questions about the example you gave above:

  1. onBarClick is specific to the Bar Chart. It means a chart can trigger different event names and we need to know all of them in order to register an event listener. Should we find a generic event ? Is an event like onClick could be sufficient ?
  2. destinationRoute and destinationParam looks like a good way to handle cases A) and B) explained above. We should either have destinationRoute or destinationParam set. What do you think ?
  3. sourceKey... I feel we should be able to add multiple params in the route: Some Elastic Search are using the name and some other are using an ID. Maybe we should be able to pass &id=X&name=my%20domain to display 2 new visualizations.

I will think deeper to this question to see how it could work !

@t00f
Copy link
Contributor

t00f commented Oct 4, 2016

Coming back on this ticket as promised.

I feel it's a flexible approach to allow the graph send some events by name and let the visualization configuration specify which event should be used.

We might need an easy way to retrieve all event names for a specific graph. Perhaps using an events.js file that lists all names as we do with ActionKeyStroke would be enough ?

The visualization configuration can then register listeners. Each listener can have a targeted action.

"listeners": [{
  "event": "onBarClick",
  "action" : {
      "type": "redirect",  // Can be "redirect" or "override" to specify the action that should be done on the URL.
      "url": "/dashboards/appDetails" // In case of "redirect", we should specify the destination url. Otherwise, the same URL is used and parameters are appended to the query part.
      "params": [  // List of params that needs to be passed in the URL (both "redirect" and "override" cases
         {
            "name": "app", // Specify the name of the param (i.e ?app=123)
            "key": "ID" // Specify the key of the object that has been clicked as you suggested previously
         },
         {
             "name": "enterprise", // We should also be able to pass a parameter that comes from the context.
             "key": "enterpriseID" // Maybe we can consider the following: If the key is not present in the object clicked, the key can be retrieved within the context ?
      ]
   }
}]

@curran curran self-assigned this Oct 10, 2016
@curran curran mentioned this issue Oct 10, 2016
28 tasks
@curran
Copy link
Contributor Author

curran commented Oct 28, 2016

@t00f I like your suggestions regarding:

  • The ability to have the event specify multiple params.
  • Drawing any unspecified parameters from the current context. I feel this probably only makes sense, though, if the destination route is the same as the current route, and only the paramerization is changing (I think you also articulated this).
  • Having a generic event, rather than "onBarClick". I think all the use cases for this navigation infrastructure are about navigating when you click on a mark inside a visualization. Let's try going for a generic "onMarkClick" event, that triggers whenever any mark is clicked (e.g. bar, pie slice, chord, table row).
  • If the key is not present in the object clicked, the key can be retrieved within the context - great idea.

I'm thinking to go with the following configuration structure, a hybrid of my original and your suggestions:

{
  "navigation": {
    "redirect": "/dashboards/appDetails",
    "params": {
      "app": "ID", // Key is the destination param, value is source column from clicked data object.
      "enterprise": "enterpriseID"
    }
  }
}

If "redirect" is unspecified, stay on current route and change params only (keeping existing params from the context).

@curran curran mentioned this issue Oct 28, 2016
@curran
Copy link
Contributor Author

curran commented Oct 31, 2016

I'm realizing that we may need multiple listeners at some point, so making the configuration an array as you suggested.

Also I do like the idea of the name "listeners" instead of "navigation", as it is more general and leaves the door open for functionality other than navigation.

{
  "listeners": [{
    "redirect": "/dashboards/appDetails", // Will override if omitted
    "params": {
      "app": "ID", // Key is the destination param, value is source column from clicked data object.
      "enterprise": "enterpriseID"
    }
  }]
}

@t00f
Copy link
Contributor

t00f commented Nov 1, 2016

Great thoughts here. I like the implementation you suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants