search

iobio / iobio-charts

0 stars 1 forks source link

Yang bam2.0 update resize observer for bamview #126

Closed YangQi007 closed 3 weeks ago

YangQi007 commented 4 weeks ago

@YangQi007 this PR is still using iobio.viz commit 27641021b5d4d92159b667ac00d8c53544f803d9. Can you update it to the latest (bab8b87b11f10478c143e66ce02dd12199a6a59d), commit on your iobio-charts branch, and push?

Done.

anderspitman commented 4 weeks ago

@YangQi007 while testing I noticed that entering a gene name is broken. It looks like we added the dynamic backend code to iobio-charts/index.html instead of bam.iobio/index.html. I should have noticed that when I reviewed it. Can you fix that in a separate PR? Once that's merged you can merge it into this branch so I can finish testing.

YangQi007 commented 3 weeks ago

@YangQi007 mostly looking good. One thing I noticed is that the brush is saved during resize in the chromosome view, but not in the global view.

I was aware of that when implementing it. In the chromosome view, I stored the region values for later use by the resizeObserver, which worked well due to the zoomToChromosomeRegion function. However, in the global view within the region component, we didn't display region values spanning multiple chromosomes. As a result, we couldn't store those values or call the zoomToChromosomeRegion function.

YangQi007 commented 3 weeks ago

@YangQi007 mostly looking good. One thing I noticed is that the brush is saved during resize in the chromosome view, but not in the global view.

After thinking about it, we can still implement it by emitting the brushed region to bamView-component and exporting a updateBrush function as I did in the updateMeanLineAndYaxis. Please let me know what you think.

anderspitman commented 3 weeks ago

Why can't BamViewChart.js store the previous brush itself? Is it because you're drawing a fresh chart every time?

YangQi007 commented 3 weeks ago

Why can't BamViewChart.js store the previous brush itself? Is it because you're drawing a fresh chart every time?

Yes, whenever the Observer detects the container's size change, the chart is redrawn based on the new container size.

anderspitman commented 3 weeks ago

It would be nice if the chart could retain its own memory and update itself, rather than redrawing from scratch every time. The web component shouldn't need to know these internal details. Would that be too big of an architectural change?

YangQi007 commented 3 weeks ago

It would be nice if the chart could retain its own memory and update itself, rather than redrawing from scratch every time. The web component shouldn't need to know these internal details. Would that be too big of an architectural change?

Are you suggesting that we update only the elements affected by the new dimensions within the SVG? If so, I think this approach would require a separate update function to manage those changes.

Yes, this would involve a significant restructuring of the code. However, I can try to extract the update logic and see how it works.

anderspitman commented 3 weeks ago

d3 in general has solid functionality for resizing things. But I don't think you're currently using those features. I think we're close enough now that it's not worth it, but something to keep in mind for the future.