monarch-initiative / monarch-app

Monarch Initiative website and API
https://monarchinitiative.org/
BSD 3-Clause "New" or "Revised" License
18 stars 6 forks source link

phenogrid multi-compare display is out of frame after monarch-app v1.8.0 #887

Open dpavam opened 1 week ago

dpavam commented 1 week ago

The Phenogrid iframe embedded on the IMPC website does not display as expected after monarch-app v1.8.0.

This is evident when a large number of phenotypes (either subjects or objectSets) are passed to the phenogrid. A good example can be found here. To open the phenogrid navigate to Human diseases caused by Pparg mutations and expand the section for Pparg-Related Familial Partial Lipodystrophy. A screenshot shows the current issue, where the labels of the objectSets are not longer visible.

Phenogrid_bad_example

The expected behaviour is achieved using the same settings but with monarch-app v1.8.0. Where all the labels are visible and users are able to scroll up or down to see the entire phenogrid. Screenshot of the expected display:

Phenogrid_good_example

I have attempted manipulating the height without success. These are the current settings being used.

// Set the iframe to fill its container
      iframe.style.width = "100%";
      iframe.style.height = "1000px";
<iframe
            ref={iframeRef}
            name="pheno-multi"
            title="MultiCompare Phenogrid"
            src="https://monarchinitiative.org/phenogrid-multi-compare"
            style={{ width: "100%", height: "100%", border: "none" }}
          />
ptgolden commented 6 days ago

I'm running git bisect from 1.8.0 onward to see where the change was introduced.

ptgolden commented 6 days ago

The behavior changed with 2c878e84

ptgolden commented 6 days ago

It's caused by removing this specific change:

https://github.com/monarch-initiative/monarch-app/commit/2c878e844bc2d10664d798570560c04fad6acc27#diff-72efdd36954cb80c05a76e4695b9fe99e3422c23a9fcb57bd2d8d226b49f4bff

Removing that class does not set overflow: auto on the SVG container, making it the default overflow: visible, which hides any part of the phenogrid that exceeds the height of its container. @kevinschaper was there a reason you removed this?

kevinschaper commented 6 days ago

My best guess is that I was looking at tables on the node page, but it's very possible that I made an irrelevant change that both broke phenogrid and didn't help the node page tables.

ptgolden commented 4 days ago

By the way @dpavam, I notice from the code that generates this phenogrid that you knowingly URL encode labels: https://github.com/mpi2/impc-mousephenotype-web/blob/1c0014040fad666cb35c214eeaa8b4b91ff61f70/components/Gene/HumanDiseases/index.tsx#L74

Can you describe what the issue is if you do not encode them? If they are showing up wrong, that's a bug on our end.

dpavam commented 4 days ago

Hi @ptgolden, this is related to #826. The mouse model identifiers (what we pass as labels to the phenogrid) contain characters such as "&", "<" or ">". If we don't encode, these characters are not shown on the tooltip.

For example: if the label is: 66.45-Dsg2<tm1a(EUCOMM)Wtsi>/Dsg2<tm1a(EUCOMM)Wtsi>

If we don't encode: the tooltip does not show the characters (and anything in between) but the label at the top is ok:

Screenshot 2024-11-15 at 18 14 59

if we encode: the tooltip shows the expected text but the label at the top is not quite right. This was a compromise we chose until the tooltip and the label have the same encoding.

Screenshot 2024-11-15 at 18 15 23
ptgolden commented 4 days ago

Ah! Thank you. I will add that that case to our testing and fix next week.

ptgolden commented 1 day ago

@dpavam the bug to watch is #902