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

Treeview migration | Review and update config properties #213

Open
RFSH opened this issue Apr 10, 2024 · 0 comments
Open

Treeview migration | Review and update config properties #213

RFSH opened this issue Apr 10, 2024 · 0 comments
Assignees
Labels

Comments

@RFSH
Copy link
Member

RFSH commented Apr 10, 2024

As part of migrating the treeview app we should also update the config properties. This app initially used hardcoded values, and the config file was created by extracting some properties. But we were not consistent in this. While some properties were moved to the config file, we still make assumptions about them (so the config file is not flexible). We've also made a lot of improvements to the config language in the plot app that we can borrow.

In this issue, I will summarize the changes we discussed that we should make. This description is not finalized and I'll update it after we discuss this more.

Details

The following are the existing main properties and what we should do about them:

title_markdown_pattern

The title_markdown_pattern property should be updated to title_display_markdown_pattern to be aligned with the plot app. The code shouldn't conditionally add this title anymore, and if we don't want this title in certain cases, we should make sure to add if cases inside the handlebars pattern.

filters

The filters property is very similar to user_controls of plot app. So we should rename it. We should also update the inner properties of this, which requires more thought and discussion.

more info should be added here

tree

The tree property is very similar to the traces property in plot-config. But we want to avoid using traces as it could imply traces in a plot. Instead, we're going to call it datasets. What goes inside the datasets is not finalized and requires more thought and discussion. In this property we need to define two urls. One is for the nodes that are involved in the tree. Another one is for the isolated nodes that don't have any parents or children. Ultimately, we want to show the union of these two requests. We must also define the node_markdown_pattern (how each node is displayed). On suggestion is the following

{
  "datasets": [
      {
        "url_patterns": ["tree_url", "isolated_nodes_url"],
        "node_markdown_pattern": "[{{Name}} ({{{ID}}})](/chaise/record/#2/Vocabulary:Anatomy/ID={{{ID}}})"
      }
    ],
}

This way, we're keeping the array notion that we have in plot-config. And then we will return the union of the url_patterns.

In the current implementation, we're defining four different URLs to simplify the patterns. Two are without the "Ordinal" filter, and two with the filter. So the suggestion above only works if we can merge these, which is something that we should explore. While discussing this, we talked about possibly allowing switch-case statements while defining patterns. This is a very rough sketch, but imagine instead of just defining a string, we could define an object like this

{
  "condition_pattern": "{{#if some condition }}true{{else}}{{/if}}",
  "main_pattern": "",
},
{
  "condition_pattern": "",
  "url_pattern": "",
},
...

annotation

For now, we decided to leave the annotation property the way it is and improve it in other phases. The only improvement I think we should make now is to change the code to use this property for generating the legend. Currently, the legend is hardcoded in the React component, which defeats the purpose of having this annotation config.

@RFSH RFSH added the treeview label Apr 10, 2024
@RFSH RFSH self-assigned this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant