-
Notifications
You must be signed in to change notification settings - Fork 45
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
COV-78 Feat/multi tab explorer #676
Conversation
|
marked it as ready again after merging a bug fix, thanks @ZakirG 👍 also added a doc as suggested by @paulineribeyre 😺 |
docs/multi_tab_explorer.md
Outdated
- `dataType`: Required. Must match the index “type” in the guppy configuration block in the manifest.json. | ||
- `nodeCountTitle`: Required. Plural of root node. | ||
|
||
For a complete list of required and optional fields for a tab configuration object, please refer to [Portal Config](https://github.com/uc-cdis/cdis-wiki/blob/master/dev/gen3/guides/ui_etl_configuration.md#portal-folder). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doc should probably be moved to this repo. it doesn't need to be private and it contains essential information for external people trying to configure windmill
what do you think about moving it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i could just copy the part that relates to portal configs over? idk if the rests (etl and manifest) should belong to here as well
src/GuppyDataExplorer/index.jsx
Outdated
chartConfig={explorerConfig[this.state.tab].charts} | ||
filterConfig={explorerConfig[this.state.tab].filters} | ||
tableConfig={explorerConfig[this.state.tab].table} | ||
heatMapConfig={this.state.tab === 0 ? dataAvailabilityToolConfig : null} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this worries me a bit. The first tab won't always be the one where we want the heatmap, and we could want different heatmaps in multiple tabs. could we move the dataAvailabilityToolConfig
to the explorerConfig
items config in a backwards compatible way? 😬the heatmap will just be another kind of chart
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right! the dataAvailabilityToolConfig
seems need some tweak. I'll fix it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can also create a ticket if it's a lot of work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a few tries, seems for now the easiest option is to put the dataAvailabilityToolConfig
as a whole inside a single tab's config
It would be cool if we can just put mainField
, aggFields
etc in the guppyConfig
for one tab, and get rid of the dataAvailabilityToolConfig
. But right now it won't work because I think if you set aggFields
for guppyConfig
the guppy wrapper will preforms sub-aggregations, and that will breaks other UI components 🤷♂️
I can create a ticket for that future improvement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, the DAT is using a separate guppywrapper so i think it's normal that it has a separate guppyconfig? but a ticket is fine :)
Co-Authored-By: Pauline Ribeyre <ribeyre@uchicago.edu>
This PR fulfills https://occ-data.atlassian.net/browse/COV-78
Allow Guppy Data Explorer to render multiple tabs as configured in the configuration file, each tab will be serving from one ES index
Deployed at https://mingfei.planx-pla.net/explorer
Added new config
explorerConfig
to repleace existingdataExplorerConfig
andfileExplorerConfig
explorerConfig
is an array which contains multiple configs (one for each explorer tab), e.g.:New Features
Deployment Changes
explorerConfig
which can take configs for multiple tabs, backward compatiable with currentdataExplorerConfig
andfileExplorerConfig
Improvements