Update orthology association table #930
Conversation
✅ Deploy Preview for monarch-app ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #930 +/- ##
==========================================
+ Coverage 71.14% 71.26% +0.11%
==========================================
Files 91 91
Lines 3136 3149 +13
==========================================
+ Hits 2231 2244 +13
Misses 905 905 ☔ View full report in Codecov by Sentry. |
| "-c", | ||
| help="A comma-separated list of counterpart categories" | ||
| ), | ||
| typer.Option("--counterpart-category", "-c", help="A comma-separated list of counterpart categories"), |
There was a problem hiding this comment.
This was autoformatting from make format
| entity_id: Annotated[ | ||
| str, typer.Argument(help="The entity to get association counts for") | ||
| ], | ||
| entity_id: Annotated[str, typer.Argument(help="The entity to get association counts for")], | ||
| fmt: fields.FormatOption = fields.OutputFormat.json, | ||
| output: fields.OutputOption = None, | ||
| ): |
| .th:nth-child(3) { | ||
| width: 30%; | ||
| } | ||
|
|
||
| /** body cells */ |
There was a problem hiding this comment.
This is where I might be causing trouble by altering potentially broader impact css, but I think I'm actually reducing the trouble that this overly broad css was causing. This PR is the first of many for defining the columns for each association type separately, and it doesn't make sense that we'll always want these widths. Plus, if you scan through Human Disease on this branch, most tables look better, in particular it gives as much space as possible to disease models.
There was a problem hiding this comment.
Yeah, we really need the new UI person to do a CSS review across the app but this looks good for now.
| :breadcrumbs="getBreadcrumbs(node, row, 'object')" | ||
| /> | ||
| </template> | ||
|
|
||
| <!-- subject --> |
There was a problem hiding this comment.
how to display an ortholog cell
| >{{ source.id }}</AppLink | ||
| > | ||
| </template> | ||
|
|
There was a problem hiding this comment.
how to display a has_evidence cell (borrowed from SectionAssociationDetails)
There was a problem hiding this comment.
You likely shouldn't be using index as a key here. If the table gets resorted, this may not be re-rendered. See the Vue documentation, and also #892 for a fix to a similar bug.
The key should uniquely identify what objcet this is rendering. I'm not totally familiar with this data structure, but I'm guessing that would be source.id.
For example, if I had the set of rows:
[
{ source: { id: 'a' }}, // index is 0
{ source: { id: 'b' }}, // index is 1
{ source: { id: 'c' }}, // index is 2
]
Vue would render if fine initially. But if I resorted those rows:
[
{ source: { id: 'c' }}, // index is 0
{ source: { id: 'b' }}, // index is 1
{ source: { id: 'a' }}, // index is 2
]
Vue would not know that it would need to re-render those rows, since the key of the elements never changes-- the first element always has index 0, the second index 1, and so on.
There was a problem hiding this comment.
fixed! thanks for catching that!
| }, | ||
| ]; | ||
| }); | ||
|
|
There was a problem hiding this comment.
which columns should be shown for the ortholog section
There was a problem hiding this comment.
I really like how you are using this data structure.
| .th:nth-child(3) { | ||
| width: 30%; | ||
| } | ||
|
|
||
| /** body cells */ |
There was a problem hiding this comment.
Yeah, we really need the new UI person to do a CSS review across the app but this looks good for now.
| }, | ||
| ]; | ||
| }); | ||
|
|
There was a problem hiding this comment.
I really like how you are using this data structure.
A first try at largely keeping our existing association table infrastructure but defining each section separately, doing the ortholog table specifically.