Upset plot, svg/charting overhaul

[vincerubinetti]

Unfortunately this became a big PR because everything here was intertwined, and I wanted to (hopefully) solve all these related issues permanently. So, the diff is very big and difficult to review, and the individual commits won't be very clean or helpful. Instead I will mark certain parts of the code to look at. And know that converting the old SVGs to use the new chart wrapper component involved a lot of copy-pasting large blocks of code. Most importantly, please re-test every visualization component on the Netlify preview.

At a high level, here are the changes:

• implement upset plot
• make generic chart wrapper component that handles most things automatically. auto-fits view box to content. SVG units match DOM units 1:1. see discussion below for how i landed on this particular design.
• abstract out download button with various download formats as its own component. integrate into chart component.
• chart text can be truncated automatically by actual rendered width (not number of chars)
• include chart titles in every chart
• include analysis id (or anything else we want) as part of every filename download. add fake analysis id to testbed page.
• turn the legend component into fully SVG instead of DOM elements. implement simple wrapping strategy.
• make MSA and IPR charts fully SVG, with no DOM elements
• MSA now auto-wraps by default, and accurately (instead of loose estimates). when printing,
• make charts resizable
• add SVG group elements where appropriate for easier editing in vector software
• consistently put settings vars and Props type at the top of each component file for quick reference
• util func simplifications and refactors

---

If you poke through the commit history, you can see some of the many complicated approaches I experimented with for handling sizing and positioning in the SVGs.

I wanted to scale the SVG shapes up/down based on the available width but keep the font size matching the document's. This was already sort of implemented before this PR, where the viz components made repeated use of a useSvgTransform hook. But it was verbose, so I first attempted to extract it out to a single hook, and then a component. It required iteratively content-fitting and font-sizing back and forth, which would (usually) converge to a stable configuration. But it was very difficult to find a foolproof way to prevent infinite re-renders or weird edge cases. I also tried leveraging simply defining font size in the root of the SVG that matched the document's, then specifying positions/sizes in terms of em units, which the browser could use synchronously with no JS. But there are some limitations to this, like not being able to use relative units in things like transforms, and some weird exceptions that caused runaway calculations like when an em-sized element exceeded the available width of the container. Trying to implement automatic text truncation made this even more complicated.

The MSA & IPR viz's also required a "fill remaining space" feature, i.e. have a column of labels on the left of a fixed width, and use all the remaining space on the right for the tracks/sequences. Another small problem to solve was that getBBox, the method used for fitting the view box to the SVG contents, doesn't work with clip-path'ed elements, which is needed for IPR's zoom area.

There's much more that I tried or thought about and didn't commit, which has already left my brain, but I think it's not that valuable.

What I landed on keeps things much simpler. The SVG units always match the DOM units, no exception (hopefully). If there isn't enough room on the page to show the SVG's content, instead of trying to shrink dynamically, it will just overflow and show scrollbars. Not only does this avoid needing iterative fitting, it's probably better UX wise since graphical elements stay in the same position and size -- instead of shrinking to an unreadable size -- and you just need to scroll over to see it all. Also this makes the testbed page much more performant. The "fill remaining" feature is accomplished by providing the available width to the consuming component, and letting it use it however it wants.

---

Regarding the MSA auto-wrapping, currently the code is basically:

setWrap(40);
print()
setWrap(oldWrap);

This is just a loose, hard-coded wrapping point that would generally not overflow the page width when printing portrait mode, US letter.

This PR doesn't change our inability to look inside the print dialog, as discussed in #50 and this comment. But I've taken out the hard-coded 40. I figure now with the resize handle available on every chart, the user can pre-size the chart to whatever they want before opening the print dialog.

I wanted to mention this because, if you look at the preview on a normal desktop or laptop screen width and have your print dialog on defaults, you'll usually see the MSA overflow the page, and I'm aware of it.

[netlify]

❌ Deploy Preview for molevolvr failed.

Name	Link
🔨 Latest commit	63a9b2d
🔍 Latest deploy log	https://app.netlify.com/projects/molevolvr/deploys/684b1d5bf25cda0008d8c1a6

[falquaddoomi]

Hey @vincerubinetti, nice work on this; it's a pretty ambitious refactor and IMHO makes common functionality a lot more streamlined for these components.

I did a little bit of testing both on the Netlify preview and running the frontend locally. All the testes were on my M1 Macbook running OS X 15.5 ("Sequoia") with the following browsers:

• Chrome (136.0.7103.114, arm64)
• Firefox (139.0.4, aarch64)

I ran into a few issues during testing, mostly with saving charts as PDFs, specifically:

• Saving a chart as PDF resets the scroll position to the top of the page
• When saving anything as a PDF and if the the MSA and/or the IPR charts are in view, their content, but not their frame, will become very narrow when the elements on the page are restored.
• I'm having a hard time reliably reproducing it, but occasionally saving as a PDF will capture the entire page, not just the selected chart. Once it happens once, it happens again for every chart I attempt to save until I refresh the page. This will cause the MSA and IPR charts to become narrow, too, as described before.
• Firefox only: the PNG and JPEG save options don't appear to do anything.

[falquaddoomi]

@vincerubinetti: Also, would it be possible to seed the RNG via a querystring parameter? It'd help to reproduce specific issues. It'd be nice if the current RNG seed were displayed on the page somewhere for copying, too, like the "Fake Analysis ID" near the top. You might even consider making it a link with the seed included for the parameter.

No worries if this is difficult, but I thought I'd ask. It is cool that it uses random data, by the way.

[vincerubinetti]

Will look into those bugs today. Seeded random shouldn't be too difficult and will help for debugging the testbed.

[falquaddoomi]

The code looks much improved from before; I can see that centralizing the charting functionality in the Chart component reduces a lot of duplicated effort across the rest of the charting components, so kudos for that.

I made a comment on the PR about some PDF issues I ran into during testing. I'm wondering if they're side effects of the DOM being changed due to the swapping to a portal containing just the element for printing (that I had just said were a good idea...)

If you need additional evidence of the bugs, I can make screen recordings and post them to the PR; just let me know.

[vincerubinetti]

Thanks for the in-depth testing and review. I hope that this is the last large scale refactor I have to do, and the experimentation I did makes me confident that this is a good way to construct custom general-purpose SVG charts in the future.

---

Also, would it be possible to seed the RNG via a querystring parameter?

Added.

Regarding the scrollbar: I guess the stakeholders have the important opinion here, but IMO the scrollbar gives you an idea of how zoomed-in you are relative to the full sequence, which I think is important for sequence browsers.

Re-added.

• Saving a chart as PDF resets the scroll position to the top of the page

Fixed.

• When saving anything as a PDF and if the the MSA and/or the IPR charts are in view, their content, but not their frame, will become very narrow when the elements on the page are restored.

Fixed. It had something to do with the fact that I was passing the chart sub-component, which had the critical containerRef attached to it, as a prop to the Download component which would then temporarily mount/unmount it as a portal, which for some reason would cause the ref to become null and mess up dimensions and everything.

I fixed it by providing a "manual" callback ref like so:

ref={(el) => {
  containerRef.current = el;
  return () => {
    containerRef.current = null;
  };
}}

But it was my understanding that this was exactly what ref={containerRef} did by default, but reading the docs linked above makes me think maybe not.

I also saw that moving all the print code and state from Download to directly within Chart, and avoiding passing the the print={chart} parameter, fixed it. But I kept the print logic in Download since I might want to use it elsewhere besides Chart at some point.

• I'm having a hard time reliably reproducing it, but occasionally saving as a PDF will capture the entire page, not just the selected chart. Once it happens once, it happens again for every chart I attempt to save until I refresh the page. This will cause the MSA and IPR charts to become narrow, too, as described before.

I couldn't replicate this, but I have a feeling it might have been fixed by the fix above.

• Firefox only: the PNG and JPEG save options don't appear to do anything.

Looks like this is occurring in the current main branch as well, and is likely due to this: bubkoo/html-to-image#508

Unfortunately, setting skipFonts just causes the image to be static noise in FF for me. So patching the package might be the only way. Or maybe going back to dom-to-image-more would fix this.

[vincerubinetti]

Unfortunately, setting skipFonts just causes the image to be static noise in FF for me. So patching the package might be the only way. Or maybe going back to dom-to-image-more would fix this.

Not sure what to do about this. At the moment, dom-to-image-more is receiving more frequent updates and better issue responses than html-to-image. But that could easily swap in the future. Also, this feels like a whack-a-mole problem, where switching might fix this issue but cause another issue in some other edge case.

Though, the APIs of these two libraries are very similar, so it should be easy to switch, so I'll try switching and see if it helps.

Also luckily for us, FF is the least popular browser of the three major ones (behind Chrome and Safari).

[falquaddoomi]

Nice! Adding in the seed is a great change, thanks for that.

Looks like the following is also fixed, too:

• scroll position is retained when saving as a PDF
• seems that that "narrowing" issue with the MSA and IPR plots is gone
• scrollbar looks good on the IPR plot
• I'm not seeing the full page when saving PDFs any more; it's just the selected chart on its own

I ran into a few more issues, though, including one I forgot to document before:

• Chrome: when saving to PDF, it seems most charts aren't centered in the page and sometimes ends up getting clipped; the only one that's centered is the heatmap. (Firefox works fine.)
  
• When zoomed in on the IPR plot, saving to PDF shows the sequence extending past its frame
  
• Selecting "Save PDF", then attempting to select any other export format for the same chart does nothing; no file is downloaded. It appears to just affect that component, not other ones on the page, and it disappears after a refresh.

Regarding Firefox, too bad that it doesn't work with that library; I see the TypeError: font is undefined error you linked in the console, just FYI. Hopefully the maintainers will resolve it, but a patch in a fork might also be a solution. (Or we just keep it as a known issue and instruct the user to use a supported browser.)

Also, definitely not necessary, but just out of curiosity is there a way to tell the browser what the PDF should be named? In Chrome it appears to get it from the page title, e.g. "Testbed _ MolEvolvR.pdf", and in Firefox it's "blank.pdf".

[vincerubinetti]

Chrome: when saving to PDF, it seems most charts aren't centered in the page and sometimes ends up getting clipped; the only one that's centered is the heatmap. (Firefox works fine.)

I can't seem to replicate this in Chrome, all of the charts appear page-centered for me. Are you using any custom settings in the print dialog?

When zoomed in on the IPR plot, saving to PDF shows the sequence extending past its frame

I was able to fix this by just moving the print controlling to the chart component itself, as I discussed above in the "callback ref" stuff. I've never seen the pattern that I was doing (assigning a component to a var with refs, and passing that component to another component as a prop to be rendered in a portal), so it's probably best to just avoid it. I imagine the error has something to do with the the order in which ref inits and cleanups are being called (or not called at all), and keeping it all in the same component fixes that. With this change, don't need the ref callbacks at all.

It's a shame I can't keep this code in the Download component to keep it general, but perhaps it's better anyway as if I'm trying to print something besides a chart, it might require different behavior or print styles anyway.

Selecting "Save PDF", then attempting to select any other export format for the same chart does nothing; no file is downloaded. It appears to just affect that component, not other ones on the page, and it disappears after a refresh.

I can't replicate this. Could you provide more detail? Maybe the latest commit fixed it? Could you try hard refreshing to make extra sure your browser isn't using old js?

Regarding Firefox, too bad that it doesn't work with that library; I see the TypeError: font is undefined error you linked in the console, just FYI. Hopefully the maintainers will resolve it, but a patch in a fork might also be a solution. (Or we just keep it as a known issue and instruct the user to use a supported browser.)

Created #58 to track.

Also, definitely not necessary, but just out of curiosity is there a way to tell the browser what the PDF should be named? In Chrome it appears to get it from the page title, e.g. "Testbed _ MolEvolvR.pdf", and in Firefox it's "blank.pdf".

It appears the only hint we can give the browser for the filename is the document title. So I've added some lines to set the title to match the filename of the other downloads, then revert it to the original title after printing.

Also I'm seeing Firefox also give the document title as the filename, not "blank".

[vincerubinetti]

Note regarding the failures: https://status.npmjs.org/

[falquaddoomi]

I thought I'd refreshed the browser, but maybe not! In any case, whether it's a combination of your commits or that, it looks like the issues I had reported are all fixed: all the figures are centered in the PDF now, the IPR sequence no longer extends past its parent frame, and the PDFs are saving with appropriate names in both Chrome and Firefox.

I'm also unable to replicate that issue I was seeing where the other export formats wouldn't work after using the PDF one, so I'll assume that's fixed, too. Just for completeness' sake, this is what I had observed in Chrome:

1. For a given chart, clicking the "PDF" option under the Download button would produce a print preview page, which I cancelled
2. After that, attempting to click any other button, i.e. "PNG", "JPEG", or "SVG", for the same chart would just do nothing. No file would be requested to be saved and nothing showed up in the console.

Anyway, it appears to be fixed now, and I haven't encountered any other issues aside from the known Firefox one with PNGs and JPEGs. Nice work!

[vincerubinetti]

I'll wait until NPM is back up before merging, as the new app wont be able to build and deploy properly until then anyway.