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

OrthoMCL - Custom tree-table for OrthoGroup page #904

Open
wants to merge 86 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
86 commits
Select commit Hold shift + click to select a range
7ac2d3e
made a start - WIP
bobular Mar 1, 2024
2f4bc1f
re-render sequence table with Mesa
bobular Mar 4, 2024
4340f10
Merge remote-tracking branch 'origin/main' into orthogroup-tree-table
bobular Mar 4, 2024
3498348
tweak comments
bobular Mar 4, 2024
dc1731f
WIP
bobular Mar 4, 2024
785a9ed
loosely wire in TreeTable and get shimmimg working
bobular Mar 5, 2024
8c3d330
add patristic and basic type support
bobular Mar 5, 2024
adb464a
made a start with controlling table row height but should investigate…
bobular Mar 8, 2024
f215762
added Mesa table tooltips for td contents when in 'inline' mode (basi…
bobular Mar 8, 2024
1abea4b
use Mesa's inline option and tidy up
bobular Mar 8, 2024
ded7a66
Revert "added Mesa table tooltips for td contents when in 'inline' mo…
bobular Mar 8, 2024
a7c0e1b
remove placeholder td title
bobular Mar 8, 2024
a19369e
restore whitespace in DataCell.tsx
bobular Mar 8, 2024
5c04215
Merge remote-tracking branch 'origin/main' into orthogroup-tree-table
bobular Mar 8, 2024
5f908bd
upgrade tidytree and use new interactive option
bobular Mar 15, 2024
89d17a7
Merge remote-tracking branch 'origin/main' into orthogroup-tree-table
bobular Mar 15, 2024
ea9283d
table and tree order are the same
bobular Mar 15, 2024
76bfe05
added basic checkboxes
bobular Mar 15, 2024
cdc9d49
WIP clustal form
bobular Mar 19, 2024
1495c07
that's hopefully clustal sorted - but can't test with local ortho-site
bobular Mar 19, 2024
57f6a10
Merge remote-tracking branch 'origin/main' into orthogroup-tree-table
bobular Mar 20, 2024
8831695
improve 'must select two proteins' verbiage
bobular Mar 20, 2024
5c896a0
de-complicate .DataTable margin-bottom override
bobular Mar 20, 2024
fd446f8
rename TreeResponse to GroupTreeResponse
bobular Mar 20, 2024
a36c6ad
add PFam domain architecture column
bobular Mar 20, 2024
f489066
ugly search working as a demo on description column
bobular Mar 20, 2024
c06178d
add warning banner for data issue and tweak pfam column heading
bobular Mar 25, 2024
2599155
used RealTimeSearchBox and made regexps safer and reorganised so we c…
bobular Mar 25, 2024
7668085
more memo and remove some commented code
bobular Mar 25, 2024
95ac689
add tree filtering
bobular Mar 25, 2024
0a15ccb
tree highlighting works with filtering now, dependency issues fixed
bobular Mar 25, 2024
0c8639b
add core/peripheral filtering
bobular Mar 25, 2024
f137fa2
added pfam legend - needs wiring still
bobular Mar 25, 2024
b4e30ed
wired up pfam checkboxes to table search
bobular Mar 25, 2024
c97bcc1
Merge remote-tracking branch 'origin/main' into orthogroup-tree-table
bobular Mar 26, 2024
0599d1b
added the row count
bobular Mar 26, 2024
6bef0de
conditionally render pfam legend and add heading
bobular Mar 26, 2024
4f1e80e
Remove vert scrollbar from tidytree table
dmfalke Mar 27, 2024
8a27952
Merge branch 'main' into orthogroup-tree-table
bobular May 29, 2024
c0cb043
remove code that collapsed the main section of the orthogroup page
bobular May 29, 2024
3e47e2c
no longer request trees for 1 or 2 sequences; fix RowCounter props ch…
bobular May 29, 2024
40eacc1
fudge to sort table based on dodgy colon-containing full_ids
bobular Jun 4, 2024
8ff24e2
make tree darker and make tree/table check more robust
bobular Jun 6, 2024
fcdddbd
provide PFam descriptions in domain cartoon tooltips
bobular Jun 6, 2024
032c479
sort out row counts
bobular Jun 6, 2024
9f3d563
Merge remote-tracking branch 'origin/main' into orthogroup-tree-table
bobular Jun 6, 2024
7bf58f4
add better console logging for tree/table mismatches
bobular Jun 11, 2024
e6904a1
made tree-table comparison for console more robust
bobular Jun 11, 2024
06f4959
add inlineUseTooltips options prop but not working fully yet
bobular Jun 19, 2024
9ecec3d
attempted better handling of very large groups
bobular Jun 19, 2024
30da70a
looks good now
bobular Jun 21, 2024
443528c
removed full_id column (from display only) and added Accession aka se…
bobular Jun 24, 2024
e7abcb9
Merge pull request #1115 from VEuPathDB/mesa-inline-changes
bobular Jun 24, 2024
9567b62
Merge remote-tracking branch 'origin/main' into orthogroup-tree-table
dmfalke Jun 28, 2024
5de59a6
Use flex for label to allow for block children
dmfalke Jun 28, 2024
2e33989
Use SelectList for Pfam filter
dmfalke Jun 28, 2024
f33f711
Merge remote-tracking branch 'origin/main' into orthogroup-tree-table
bobular Jul 2, 2024
5e04d5f
Explicit empty PFam legend
bobular Jul 2, 2024
5a49184
Merge branch 'main' into orthogroup-tree-table
bobular Jul 2, 2024
5cf07bf
Use similar style for core/peripheral filter
dmfalke Jul 12, 2024
0cbb71d
Merge remote-tracking branch 'origin/main' into orthogroup-tree-table
dmfalke Jul 12, 2024
592ef2d
Merge branch 'orthogroup-tree-table' into orthogroup-tree-table__pfam…
dmfalke Jul 12, 2024
b316022
Add species filter to protein table
dmfalke Jul 16, 2024
a422072
prevent overflow in SelectTree button with many items selected; also …
bobular Jul 17, 2024
c50a85e
add shouldOnlyUpdateOnClose option to SelectTree
bobular Jul 17, 2024
96056a2
remove some unused imports
bobular Jul 17, 2024
8c05b0c
add MAX_SEQUENCES_FOR_TREE logic
bobular Jul 17, 2024
0620543
selected row highlighting
bobular Jul 17, 2024
871583e
tone down the row background color
bobular Jul 17, 2024
f00a872
Merge pull request #1140 from VEuPathDB/orthogroup-tree-table__pfam-f…
bobular Jul 18, 2024
6d50fca
Merge remote-tracking branch 'origin/main' into orthogroup-tree-table
bobular Jul 18, 2024
11deb34
don't indent search bar and filters
bobular Jul 19, 2024
a718914
add instantUpdate option to SelectList
bobular Jul 19, 2024
d168e75
Merge remote-tracking branch 'origin/orthogroup-tree-table' into orth…
bobular Jul 19, 2024
9c5ff46
revert taxon filter to instant-update
bobular Jul 19, 2024
de9d761
instantUpdate for core/peripheral filter
bobular Jul 19, 2024
9281471
Pfam architectures are now scaled by protein length
bobular Jul 19, 2024
789629b
removed core-only filter for larger groups
bobular Jul 19, 2024
209fd02
improved table sort and filter performance - added comments about fur…
bobular Aug 1, 2024
3874c5a
rename shouldOnlyUpdateOnClose to instantUpdate
bobular Aug 2, 2024
d45f975
remove an effect
bobular Aug 2, 2024
f9a19b7
Merge remote-tracking branch 'origin/orthogroup-tree-table' into orth…
bobular Aug 2, 2024
eb238e5
missed a setSelected and dependency
bobular Aug 2, 2024
66e13f2
final tweaks to button label behaviour
bobular Aug 2, 2024
969db0d
Merge pull request #1144 from VEuPathDB/ortho-instant-filters
bobular Aug 2, 2024
febb312
add reset button, revisit filter button layout
bobular Aug 3, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/libs/components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"react-spring": "^9.7.1",
"react-transition-group": "^4.4.1",
"shape2geohash": "^1.2.5",
"tidytree": "github:d-callan/TidyTree"
"tidytree": "https://github.com/d-callan/TidyTree.git#commit=9063e2df3d93c72743702a6d8f43169a1461e5b0"
},
"files": [
"lib",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useLayoutEffect, useRef } from 'react';
import { CSSProperties, useEffect, useLayoutEffect, useRef } from 'react';
import { TidyTree as TidyTreeJS } from 'tidytree';

export interface HorizontalDendrogramProps {
Expand All @@ -19,6 +19,7 @@ export interface HorizontalDendrogramProps {
for now just default to all zero margins (left-most edges
*/
margin?: [number, number, number, number];
interactive?: boolean;
};

/// The remaining props are handled with a redraw: ///
Expand All @@ -31,10 +32,21 @@ export interface HorizontalDendrogramProps {
* width of tree in pixels
*/
width: number;
/**
* hopefully temporary prop that we can get rid of when we understand the
* horizontal layout behaviour of the tree (with respect to number of nodes)
* which will come with testing with more examples. Defaults to 1.0
* update: possibly wasn't needed in the end!
*/
hStretch?: number;
/**
* number of pixels height taken per leaf
*/
rowHeight: number;
/**
* CSS styles for the container div
*/
containerStyles?: CSSProperties;
/**
* which leaf nodes to highlight
*/
Expand All @@ -57,9 +69,11 @@ export function HorizontalDendrogram({
leafCount,
rowHeight,
width,
options: { ruler = false, margin = [0, 0, 0, 0] },
options: { ruler = false, margin = [0, 0, 0, 0], interactive = true },
highlightedNodeIds,
highlightMode,
hStretch = 1.0,
containerStyles,
}: HorizontalDendrogramProps) {
const containerRef = useRef<HTMLDivElement>(null);
const tidyTreeRef = useRef<TidyTreeJS>();
Expand All @@ -80,7 +94,9 @@ export function HorizontalDendrogram({
equidistantLeaves: true,
ruler,
margin,
hStretch,
animation: 0, // it's naff and it reveals edge lengths/weights momentarily
interactive,
});
tidyTreeRef.current = instance;
return function cleanup() {
Expand Down Expand Up @@ -119,6 +135,7 @@ export function HorizontalDendrogram({
style={{
width: width + 'px',
height: containerHeight + 'px',
...containerStyles,
}}
ref={containerRef}
/>
Expand Down
39 changes: 31 additions & 8 deletions packages/libs/components/src/components/tidytree/TreeTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,13 @@ export interface TreeTableProps<RowType> {
/**
* number of pixels vertical space for each row of the table and tree
* (for the table this is a minimum height, so make sure table content doesn't wrap)
* required; no default; minimum seems to be 42; suggested value: 45
*/
rowHeight: number;
/**
* number of pixels max width for table columns; defaults to 200
*/
maxColumnWidth?: number;
/**
* data and options for the tree
*/
Expand Down Expand Up @@ -42,16 +47,25 @@ export interface TreeTableProps<RowType> {
* - allow additional Mesa props and options to be passed
*/
export default function TreeTable<RowType>(props: TreeTableProps<RowType>) {
const { rowHeight } = props;
const { rowHeight, maxColumnWidth = 200 } = props;
const { rows } = props.tableProps;

const rowStyleClassName = useMemo(
() =>
cx(
classNameStyle({
height: rowHeight + 'px',
background: 'yellow',
})
// minimum height for table rows
classNameStyle`
height: ${rowHeight}px;

& td {

&:hover {
cursor: pointer;
position: relative;
}

}
`
),
[rowHeight]
);
Expand All @@ -63,6 +77,9 @@ export default function TreeTable<RowType>(props: TreeTableProps<RowType>) {
options: {
...props.tableProps.options,
deriveRowClassName: (_) => rowStyleClassName,
inline: true,
inlineMaxHeight: `${rowHeight}px`,
inlineMaxWidth: `${maxColumnWidth}px`,
},
};

Expand All @@ -74,18 +91,24 @@ export default function TreeTable<RowType>(props: TreeTableProps<RowType>) {
{...props.treeProps}
rowHeight={rowHeight}
leafCount={rows.length}
options={{ margin: [0, 10, 0, 10] }}
options={{ margin: [0, 10, 0, 10], interactive: false }}
/>
<>
<div
style={{
flexGrow: 1,
width: 1 /* arbitrary non-zero width seems necessary for flex */,
}}
>
<Global
styles={globalStyle`
.DataTable {
margin-bottom: 0px !important;
width: 80%;
}
`}
/>
<Mesa state={tableState} />
</>
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm just noticing this. I'm not sure we should use Global here, as the style will be reflected on all subsequent pages.

Instead, could you use a css prop on the parent div component (line 96), and target .DataTable in there?

      <div
        css={{
          flexGrow: 1,
          width: 1 /* arbitrary non-zero width seems necessary for flex */,
          '.DataTable': {
            marginBottom: '0px !important',
            width: '80%'
          }
        }}
      >
        <Mesa state={tableState} />
    </div>

Copy link
Member Author

@bobular bobular Mar 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, that's much simpler (and no side effects) - done in 5c896a0 - thanks!

</div>
);
}
5 changes: 4 additions & 1 deletion packages/sites/ortho-site/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -89,5 +89,8 @@
"files": [
"dist",
"webapp"
]
],
"dependencies": {
"patristic": "^0.6.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be declared in packages/libs/components/package.json.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's

import { parseNewick } from 'patristic';

in Sequences.tsx though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use patristic directly in components. If tidytree needs it then that's its business!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, thanks!

}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import React, { useState } from 'react';
import TreeTable from '@veupathdb/components/lib/components/tidytree/TreeTable';
import { RecordTableProps, WrappedComponentProps } from './Types';
import { useOrthoService } from 'ortho-client/hooks/orthoService';
import { Loading } from '@veupathdb/wdk-client/lib/Components';
import { parseNewick } from 'patristic';
import { AttributeValue } from '@veupathdb/wdk-client/lib/Utils/WdkModel';

type RowType = Record<string, AttributeValue>;

export function RecordTable_Sequences(
props: WrappedComponentProps<RecordTableProps>
) {
const groupName = props.record.id.find(
({ name }) => name === 'group_name'
)?.value;

if (!groupName) {
throw new Error('groupName is required but was not found in the record.');
}

const treeResponse = useOrthoService(
(orthoService) => orthoService.getGroupTree(groupName),
[groupName]
);

const [highlightedNodes, setHighlightedNodes] = useState<string[]>([]);

if (treeResponse == null) return <Loading />;

const mesaColumns = props.table.attributes
.map(({ name, displayName }) => ({
key: name,
name: displayName,
}))
// and remove a raw HTML checkbox field - we'll use Mesa's built-in checkboxes for this
// and an object-laden 'sequence_link' field - the ID seems to be replicated in the full_id field
.filter(({ key }) => key !== 'clustalInput' && key !== 'sequence_link');

const mesaRows = props.value;

// do some validation on the tree w.r.t. the table

// should this be async? it's potentially expensive
const tree = parseNewick(treeResponse.newick);
const leaves = tree.getLeaves();

// sort the table in the same order as the tree's leaves
const sortedRows = leaves
.map(({ id }) => mesaRows.find(({ full_id }) => full_id === id))
.filter((row): row is RowType => row != null);

if (leaves.length !== sortedRows.length)
return (
<div>Tree and protein list mismatch, please contact the helpdesk</div>
);

const mesaState = {
options: {
isRowSelected: (row: RowType) =>
highlightedNodes.includes(row.full_id as string),
},
rows: sortedRows,
columns: mesaColumns,
eventHandlers: {
onRowSelect: (row: RowType) =>
setHighlightedNodes((prev) => [...prev, row.full_id as string]),
onRowDeselect: (row: RowType) =>
setHighlightedNodes((prev) => prev.filter((id) => id !== row.full_id)),
},
};

const treeProps = {
data: treeResponse.newick,
width: 200,
highlightMode: 'monophyletic' as const,
highlightedNodeIds: highlightedNodes,
};

const rowHeight = 45;

return (
<>
<div>Ignore the help text above for now!</div>
<TreeTable
rowHeight={rowHeight}
treeProps={treeProps}
tableProps={mesaState}
/>
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import {
groupLayoutResponseDecoder,
} from 'ortho-client/utils/groupLayout';
import { TaxonEntries, taxonEntriesDecoder } from 'ortho-client/utils/taxons';
import { TreeResponse, treeResponseDecoder } from 'ortho-client/utils/tree';

export function wrapWdkService(wdkService: WdkService): OrthoService {
return {
...wdkService,
getGroupLayout: orthoServiceWrappers.getGroupLayout(wdkService),
getGroupTree: orthoServiceWrappers.getGroupTree(wdkService),
getProteomeSummary: orthoServiceWrappers.getProteomeSummary(wdkService),
getTaxons: orthoServiceWrappers.getTaxons(wdkService),
};
Expand All @@ -26,6 +28,12 @@ const orthoServiceWrappers = {
method: 'get',
path: `/group/${groupName}/layout`,
}),
getGroupTree: (wdkService: WdkService) => (groupName: string) =>
wdkService.sendRequest(treeResponseDecoder, {
useCache: true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth noting that this property causes the response to be stored in indexeddb, which is governed by the browsers quota policy for domains. If the database exceeds the quota, attempts to cache responses will begin to fail (don't worry, this is handled for a seamless user experience). In all likelihood, important/critical responses will already be cached, so it should not impact website load times.

In the future, we might consider using LRU (least recently used) caches for things like this, where the cache container has an identifier, and we retain up to a certain number of entries, ejecting those used least recently.

For now, this is probably fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Are the cache entries keyed by the full request payload (like react-query)?

If so, this would seem to be a general WDK problem, not specific to fetching the tree (is it massively bigger than the responses from the other endpoints?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entries are keyed by path and, if provided, cacheId. See here. I don't know how big these newick strings can get, so it's hard to say how much of an issue it would be.

Again, I think this is probably fine, but I wanted to mention the current limitations.

method: 'get',
path: `/newick-protein-tree/${groupName}`,
}),
getProteomeSummary: (wdkService: WdkService) => () =>
wdkService.sendRequest(proteomeSummaryRowsDecoder, {
useCache: true,
Expand All @@ -42,6 +50,7 @@ const orthoServiceWrappers = {

export interface OrthoService extends WdkService {
getGroupLayout: (groupName: string) => Promise<GroupLayoutResponse>;
getGroupTree: (groupName: string) => Promise<TreeResponse>;
getProteomeSummary: () => Promise<ProteomeSummaryRows>;
getTaxons: () => Promise<TaxonEntries>;
}
Expand Down
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be moved to packages/libs/components

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
declare module 'patristic' {
export class Branch {
id: string;
parent?: Branch | null;
length?: number;
children?: Branch[];
value?: number;
depth?: number;
height?: number;

constructor(data: Branch, children?: (data: any) => Branch[]);
addChild(data: Branch): Branch;
addParent(data: Branch, siblings?: Branch[]): Branch;
ancestors(): Branch[];
clone(): Branch;
getLeaves(): Branch[];
}

export function parseNewick(newickStr: string): Branch;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we change these identifiers such that they include the "group" prefix? E.g., GroupTreeResponse, groupTreeResponseDecoder, etc?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! fd446f8

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { Decoder, record, string } from '@veupathdb/wdk-client/lib/Utils/Json';

export interface TreeResponse {
newick: string;
}

export const treeResponseDecoder: Decoder<TreeResponse> = record({
newick: string,
});
Loading
Loading