search

materialsproject / crystaltoolkit

Crystal Toolkit is a framework for building web apps for materials science and is currently powering the new Materials Project website.
https://docs.crystaltoolkit.org
Other
155 stars 59 forks source link

Fix `StructureMoleculeComponent` legend if same element has multiple oxi states #339

Closed janosh closed 1 year ago

janosh commented 1 year ago

Closes #338.

This is a WIP to start gathering feedback. As suggested by @yang-ruoxi, this first step splits every oxi state into its own legend label without affecting the site colors. I.e. different oxi states still have the same color in the structure.

Suggestion: Make negative oxi states slightly darker, positive ones slightly brighter and combine different oxi states for the same element into 1 wider label.

This PR requires upstream changes in pymatgen: https://github.com/materialsproject/pymatgen/pull/2998 i.e. we can't put this into prod until pymatgen cuts a new release.

@mkhorton If you have a minute, could you let me know if there's any problem with making StructureMoleculeComponent.get_scene_and_legend() non-static? See e418cae.

Here's what this PR looks like for the problematic structure reported by @ardunn on matsci.org.

Screenshot 2023-05-17 at 18 03 24

yang-ruoxi commented 1 year ago

thanks @janosh !! It looks nice. I can see downstream people ask for color distinction for different oxi states, but as an incremental fix for the labels I think this is sufficient.

janosh commented 1 year ago

Sure happy to merge as is. We just need to await the next pymatgen release. Also, would be great if @mkhorton or @tschaume or whoever can invite me to the Percy project to green-light the visual changes causing CI to fail.

percy.io Screenshot 2023-05-22 at 11 00 25

janosh commented 1 year ago

New pymatgen release to go along with this merge planned for later this week.