Heatmap component #52
Conversation
✅ Deploy Preview for molevolvr ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Nice work! The heatmap looks good, and the following functions all seem to work as expected in Chrome/Firefox on OS X:
- hovering over a heatmap cell shows its value and metadata
- hovering over an axis label displays the full label text
- gradient, flip, transpose controls work as expected
- tabbing through both cells and labels works as expected
- tabbing through and interacting with controls using the keyboard works, including help tooltips
- all the download options work (kudos for making the PNGs alpha-transparent)
One extremely minor thing is that the PDF download produces two pages on OS X in Chrome/Firefox, one with the plot and one blank page.
Safari works aside from the following issues (Version 18.4 (20621.1.15.11.10)):
- the PDF download is just one page, but it seems the figure isn't centered in the page and much of it is clipped, including the gradient scale:
Testbed | MolEvolvR.pdf - tabbing through the cells and axis labels work, but least in the Netlify preview I can't seem to reach the controls via tabbing; they don't appear to receive keyboard focus. The gradient dropdown can be navigated via the keyboard once expanded, though.
Trying to fix this, but it's proving difficult. A style change to
This is due to this unfortunate default behavior on MacOS: https://stackoverflow.com/a/67901876/2180570 More "regular" controls like inputs can be focused with tab by default, but not buttons and other things. And since our custom components are ultimately button elements in the DOM, this happens. |
|
I've made some changes that should improve Safari printing a bit, and not mess up any other figure PDF printing in other browsers (I've checked each manually). It genuinely seems that Safari does not respect/understand viewport CSS units when printing, so idk how much I'm able to fix here. Take this very basic example<html>
<head>
<meta name="viewport" content="width=device-width,initial-scale=1.0" />
<style>
body {
margin: 0;
}
div {
background: red;
width: 100vw;
height: 100vh;
}
</style>
</head>
<body>
<div></div>
</body>
</html>(Turn on "print backgrounds"). I can't get the red box to perfectly fix the page width/height (for arbitrary page size and landscape/portrait), no matter what CSS I use. Maybe this very new CSS feature could help, idk https://developer.mozilla.org/en-US/docs/Web/CSS/@page/size |
|
It seems like what you changed fixed multiple pages appearing in Chrome/Firefox PDF downloads, and it definitely looks better in Safari! 🎉 Thanks for letting me know about the tabbing fix for Safari; frankly, I don't normally test with Safari, so I didn't occur to me that it might be an OS/browser issue. FWIW, regarding your SO link, I ended up being able to just enable this value in the Safari settings for tabbing to work with all the controls; I didn't need to change the OS-level settings: That's too bad about Safari's incomplete support for viewport units, but like you said I think a best-effort attempt with the time we have is good enough. Generally, as long as everything works in one major browser, we can make a best effort for the rest and add a disclaimer that everything might not work as well as it does in the chosen one. In this case they have a variety of other formats that they could easily convert to PDF, so I don't think totally consistent PDF support across browsers is critical. I do appreciate your effort to get closer to that, though! 🙂 |
|
Appreciate the safari fix as well. The heatmap looks great! 👌🏼 |
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 `transform`s, 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: ```ts 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](#52 (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.
Closes #37