cbouy / mols2grid

Interactive molecule viewer for 2D structures
https://mols2grid.readthedocs.io
Apache License 2.0
206 stars 25 forks source link

Responsive grid & UI/UX changes #52

Closed themoenen closed 1 year ago

themoenen commented 1 year ago

Refactored improvements

Small improvements

Bug fixes

cbouy commented 1 year ago

I've just pushed some changes to fix the issue with the selection not working (as well as Python callbacks and filtering). Basically, without a call to display(widget), the widget that controls the interaction between JavaScript and Python is never properly initialized and so all these features stop working.

Here are my comments after playing with it a bit, haven't looked too much at the code yet.

Major

Minor

Nitpick

And just keeping notes for myself: it might be worth having a dark_mode parameter to switch the font-color to white, and the default color palette for the molecule rendering to white, so that everything looks good for dark mode users.

cbouy commented 1 year ago

Alright all of the tests should be fixed now (hopefully 🤞)

cbouy commented 1 year ago

Sorry looks like GitHub does not like me doing a git push --force on your branch after rebasing my commits to clean up the git history a bit... I'll reopen

cbouy commented 1 year ago

Mmmh looks like I need to create a new branch since I can't push changes to your branch when the PR has been closed. I guess I'll make a PR to your mols2grid-responsive fork, then once you merge it you'll be able to reopen a new PR here and make changes to it directly. I'm really sorry for the inconvenience 😅