Ngl SideBySide fix and Mol3d isosurface/shapes

[HammadTheOne]

Closes #526
Closes #518

About

• I am closing an issue
• I am adding a feature to an existing component, or improving an existing feature

Description of changes

This PR adds the isosurfaces and shapes props to the Molecule3dViewer component, and fixes an issue with sideByside not working correctly with ALL chains selected in NglMoleculeViewer.

• Fix ngl sidebyside fb4ffa1
• Add isosurfaces and shapes c77d79b
• Fix integration tests and Percy snapshots flakiness

TODO:

• Add new feature documentation to dash-docs (https://github.com/plotly/dash-docs/pull/1107)

[alexcjohnson]

Can we document this as part of the propType? So instead of labels: PropTypes.array we would have something like (if I'm understanding this correctly):

/**
 * Labels corresponding to the atoms of the molecule.
 */
labels: PropTypes.arrayOf(PropTypes.exact({
    /**
     * The label content
     */
    text: PropTypes.string,

    /**
     * additional styling options. See <docs url> for details
     */
    parameters: PropTypes.object
}))

Notice I used exact - this is preferred over shape unless it's specifically desirable to allow arbitrary extra keys in the object. Should we change this for orbital below, or does it support extra keys?

[HammadTheOne]

For orbital, while it doesn't support extra keys, the component itself supplies defaults for missing keys. I think changing it to exact is fine, if the warning for additional or missing keys is what we're looking to have. Fixed in 078ebc7.

[alexcjohnson]

exact is fine, if the warning for additional or missing keys is what we're looking to have

exact doesn't warn on missing keys - that's still fine unless the individual key has .isRequired. All it does is forbid including additional keys. So yes, this is what we want.

[HammadTheOne]

After looking at it again, the parameters are just input as extra keys to the label object. In 3010db8 I changed it to remove the explicit parameters prop and changed it back to shape so arbitrary keys can be passed in without warnings. Does that make sense for this prop?

[alexcjohnson]

See below #547 (comment) - FYI all of this would be clearer if there were a test using the labels key so we could both be looking at the same behavior 😉

[HammadTheOne]

I added a Percy snapshot test in the last couple of commits. One thing I'm confused about is that the Percy build isn't showing the new screenshot for approval in the build. Does that require manual setup aside from just adding in dash_duo.percy_snapshot('test-mol3d_labels') to the test?

[alexcjohnson]

oh boy... we have a problem, all the percy snapshots are getting separate builds on their side. Check out https://percy.io/plotly/dash-bio and see all the ones from the same commit. Try using the percy-finalize pattern like we use elsewhere, see eg plotly/dash-html-components@72796c4

[HammadTheOne]

OK, after a long series of commits, the tests are finally consolidated into one build with percy-finalize, and running successfully on 3.7 after fixing flakiness for a few of them.

One thing I am noticing for the Percy snapshots is that the molecules don't seem to be rendered for the mol-3d and ngl-moleculeviewer tests. I tried adding in a delay with a check using wait_for_text_to_equal and a dummy div, but it seems to be the same case, despite loading fine locally.

Do you have any suggestions on how to make these snapshots more reliable?

[alexcjohnson]

Excellent, nice job! The unrendered pieces are probably in canvases (WebGL or otherwise) - you'll need convert_canvases in the snapshot call, like here:

https://github.com/plotly/dash/blob/e8ac94919105a91c76a966c21aca2ec7b0297e22/tests/integration/devtools/test_devtools_ui.py#L103

[HammadTheOne]

Thanks, that worked perfectly, and now all but the Speck ones are generated successfully! I can't readily see what might be causing it since Speck is also a WebGL rendered component, and locally it doesn't seem like there's a problem, but I think it might be worth addressing that in a separate PR and wrapping this one up (I feel like it's gotten a little out of scope already 😅 ).

[HammadTheOne]

@alexcjohnson Tests successfully running again 🚀

[alexcjohnson]

Excellent! 💃