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

[Feature Request]: Simplify tooltip configuration process #1292

Open
wadhwamatic opened this issue Jun 28, 2024 · 2 comments
Open

[Feature Request]: Simplify tooltip configuration process #1292

wadhwamatic opened this issue Jun 28, 2024 · 2 comments
Assignees
Labels
enhancement New feature or request triage to be triaged for next action

Comments

@wadhwamatic
Copy link
Member

Provide a clear and concise description of what you want to happen.

We improved the functionality of the tooltip over time, including most recently through PR #1286. But the configuration process and many options that exist is getting complicated.

This issue is intended to capture thoughts on how we can simplify the configuration process so that future PRISM configs are straightforward to implement.

My suggestion is to replace feature_info_props with tooltip and to move all information that is displayed in the tooltip here. Please have a look at the example below including the comments

"tooltip": {
  "primary_field": {
    "title": "layer_title",
    // by default the title is the layer's title but it can be modified here. this should be an optional config parameter and layer_title is reserved text. 
    "source": "layer_value",
    // for the primary field, options are layer_value or legend_label. This allows us to use numeric values or strings which is useful for either continuous or categorical variables
  },
  "additional_fields": {
    "title": "Population",
    "source": "pop",
    // an additional field can be configured from the data source of the layer. In this example, there's a field called 'pop' within this layer's json data source
    "type": "number"
  },
  "additional_fields": {
    "title": "Risk index",
    "source": "mdr_index.multi_dim_risk_index",
    // in this example, the source is in another layer called mdr_index, and a field in that layer called multi_dim_risk_index. this is a new feature request
    "type": "number"
  },
  "displayAdminLevel": false
  // feature request to optionally show/hide the admin level in the tooltip
}

@ericboucher and @gislawill - please add your inputs here as well.

Is there anything else you can add about the proposal? You might want to link to related issues here, if you haven't already.

No response

@gislawill
Copy link
Collaborator

Thanks @wadhwamatic for putting this together! A couple thoughts:

  1. It'd be great to layout how we'd support tables and charts in this configuration as well. Is it clear to you when we display tables and charts within the current logic?
    1. I see special handling for "Population in phase 1" tables in the ToolTip code
    2. I see a PopupChartsList and PopupAnalysisCharts that are displayed when the adminLevel is not defined but it's not clear to me in the code when these should show
  2. We currently call the Tooltip a "Popup" in our React code, I think this is a good opportunity to update it to "Tooltip" as well.

Last note, I think we should make this slight update to what you had before that treats "additional_fields" as an array rather than multiple fields with the same key:

"tooltip": {
  "primary_field": {
    "title": "layer_title",
    // by default the title is the layer's title but it can be modified here. this should be an optional config parameter and layer_title is reserved text. 
    "source": "layer_value",
    // for the primary field, options are layer_value or legend_label. This allows us to use numeric values or strings which is useful for either continuous or categorical variables
  },
  "additional_fields": [
    { 
      "title": "Population",
      "source": "pop",
      // an additional field can be configured from the data source of the layer. In this example, there's a field called 'pop' within this layer's json data source
      "type": "number"
    },
    {
      "title": "Risk index",
      "source": "mdr_index.multi_dim_risk_index",
      // in this example, the source is in another layer called mdr_index, and a field in that layer called multi_dim_risk_index. this is a new feature request
      "type": "number"
    },
  ],
  "displayAdminLevel": false   // feature request to optionally show/hide the admin level in the tooltip
}

@wadhwamatic
Copy link
Member Author

Thanks @gislawill for the feedback and additional thinking on this.

Very good points about the special handling of population phase, PopupChartsList and PopupAnalysisCharts. I don't know the details of how this was implemented, but we should make this more clear within work for this issue, and consider how we could optionally use tables / charts in this config process but keeping in mind the goal of simplifying things. @ericboucher - thanks for input on this.

Yes, agreed, let's make our terms consistent. We might want to specify this as the admin area tooltip, since we also have a tooltip on the timeline.

Also agreed to make additional_fields an array - good catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triage to be triaged for next action
Projects
None yet
Development

No branches or pull requests

2 participants