Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Synchronize all parts (molstar, viewer, protvista) by AUTH chainId instead of STRUCT asymId #238

Open
wants to merge 23 commits into
base: development
Choose a base branch
from

Conversation

p3rcypj
Copy link

@p3rcypj p3rcypj commented Dec 20, 2024

📌 References

📝 Implementation

  • Changed all STRUCT asym id references for chains, to AUTH chain id
  • Sync with SequenceViewer on chain is changed, and when ligand is selected
  • Fixed sync with protvista
  • Emdb is now load at the first init along with pdb, in order to not make selection re-update
  • Chains dropdown is now disabled in ligand view
  • Ligand dropdown now will appear always but disabled whenever that chain does not have ligands
  • Minor fix: EBI is returning duplicated entries in search -> removed duplicates from response

@p3rcypj p3rcypj requested review from adrianq and tokland December 20, 2024 14:01
Copy link

@tokland tokland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In-line comments:

@p3rcypj
Copy link
Author

p3rcypj commented Jan 19, 2025

I rewrote most of the responsibilities on usePluginRef, I hope now it makes more sense by reading it. Abstracting it into smaller functions was a really good point because it made me review all the scenarios again several times and when the code was really executed I ended up removing unnecessary code. Thanks!!

@p3rcypj p3rcypj requested a review from tokland January 19, 2025 06:02
Copy link

@tokland tokland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, code-wise 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants