VEuPathDB / web-monorepo

A monorepo that contains all frontend code for VEuPathDB websites
Apache License 2.0
2 stars 0 forks source link

Orthogroup tree table perf #1233

Closed dmfalke closed 1 month ago

dmfalke commented 1 month ago

This PR addresses some performance issues with the protein TreeTable on group pages.

dmfalke commented 1 month ago

I still need to address the issue where the page jumps when a filter is applied.

bobular commented 1 month ago

From ui-infra

Verbose help text should be replaced by one sentence and "read more..." toggle.

Replace "Note" with a warning icon :warning:

bobular commented 1 month ago

Run clustal seems greyed out

bobular commented 1 month ago

Don't forget the vertical placement of the tree w.r.t. table (it has gone out of sync a bit)

bobular commented 1 month ago

I had another play with useDeferredValue.

This approach seems to reduce the checkbox delay considerably (but not entirely)

  const [volatileCorePeripheralFilterValue, setCorePeripheralFilterValue] = useState<
    ('core' | 'peripheral')[]
  >([]);
  const corePeripheralFilterValue = useDeferredValue(volatileCorePeripheralFilterValue);

and then only use the "volatile" value in the SelectList.

The page/table moving around underneath the SelectList widget isn't ideal, as you already mentioned.

bobular commented 1 month ago

Maybe even this?

import { useState, useDeferredValue } from 'react';

export function useDeferredState<T>(initialValue: T): [T, T, React.Dispatch<React.SetStateAction<T>>] {
  const [volatileValue, setVolatileValue] = useState<T>(initialValue);
  const deferredValue = useDeferredValue(volatileValue);

  return [volatileValue, deferredValue, setVolatileValue];
}
bobular commented 1 month ago

Verbiage suggestion:

Currently

To see a phylogenetic tree please use a filter to display fewer than 1,000 sequences

Could be

To see a phylogenetic tree please use the filters to restrict to fewer than 1,000 sequences

dmfalke commented 1 month ago

@bobular thanks for the feedback and suggestions. I am going to merge this as it currently is. I think we can explore useDeferredValue and updating verbiage in a new PR.