search

phenopolis / phenopolis_genomics_browser

Python API and React frontend for the Phenopolis Genomics Browser
https://dev-live.phenopolis.org
MIT License
31 stars 2 forks source link

Base table #310

Closed YuanTian1991 closed 3 years ago

YuanTian1991 commented 3 years ago

The base table is a bit slow than I though. Here the PR is only used to create a netlify preview, NOT for merge.

We need to decide if we want to use this BaseTable.

The advantage of BaseTable:

  1. Easy to use, much easier than react-window. Like, modify cell, freeze column .etc
  2. Code is cleaner, the old table has 1000 line of code.

The disadvantage of BaseTable:

  1. It renders all columns (NOT row) all at once, the old table is "dynamically" render column and row. So maybe it's the reason it's slower when I scroll.
netlify[bot] commented 3 years ago

Deploy preview for phenopolis-dev ready!

Built with commit 31ea27269b7816fe20dd93d52082e5b65d819fcc

https://deploy-preview-310--phenopolis-dev.netlify.app

IsmailM commented 3 years ago

Ah, that's a real shame - I had assumed it would also do column virtualization...

There is an open issue on this: https://github.com/Autodesk/react-base-table/issues/36

The issue has quite a few comments with the author and even includes exemplar code (e.g. this) showing how to implement column visualisation - may be worth it to go through this issue in detail and look at all the code examples...

Happy to have a chat if you want to discuss this further

YuanTian1991 commented 3 years ago

I tried this example and it somewhat works.

However, it would "dynamically" estimate rowHeight AFTER I horrizentally scroll, which means for same row, when I scroll left or right. The height would sometimes high (with many chips), sometimes low (with no chips).

There is MAYBE a way to get over above problem: pre-calculate each row's height (like I did before), save it into a array. Then use estimatedRowHeight function to get this pre-calculated row-height when scroll left/right.

Also, seems it requires a "tableWidth" parameter, which can't be 100%. A way maybe can get over it is to use some web-width estimate library to get "real pixel" value of monitor, then assign this value as tableWidth.

Do you think it worth a try?

pontikos commented 3 years ago

hey @YuanTian1991 just following up, do you want to give us a demo at some point?

YuanTian1991 commented 3 years ago

Hi @pontikos You can check the demo anytime on the Netlify generated PR website.

For example, https://deploy-preview-310--phenopolis-dev.netlify.app/gene/ENSG00000119685 this link shows current progress on the new table.

pontikos commented 3 years ago

@YuanTian1991 this is looking amazing! It basically works right? Anything outstanding except for the issue raised above?

YuanTian1991 commented 3 years ago

I have not fully test is, for example currently the new table only works for Gene page. So at least there are 3 main work to be done:

  1. Apply this new table to all pages, totally replace old table.
  2. Make the Plot function work.
  3. Make the Genome function work.

There are some issues related to Plots, I will try to solve them as well during this development.