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

#5700 - System allow to establish infinit number of bonds from monomer to microstructure #6225

Conversation

lmhs
Copy link
Collaborator

@lmhs lmhs commented Dec 28, 2024

How the feature works? / How did you fix the issue?

Following the proposal by @tanas80 and further discussion with @AlexeyGirin and @ljubica-milovic it was approved that to resolve issue #5700 creation of non-hydrogen bond from center of the CHEM monomer must be forbidden with notification banner displayed if bond target is Atom, only attachment points can be used for creating bonds from CHEM monomer to Atoms

Fixes #5700

Check list

  • unit-tests written
  • e2e-tests written
  • documentation updated
  • PR name follows the pattern #1234 – issue name
  • branch name doesn't contain '#'
  • PR is linked with the issue
  • base branch (master or release/xx) is correct
  • task status changed to "Code review"
  • reviewers are notified about the pull request

Sorry, something went wrong.

@lmhs lmhs self-assigned this Dec 28, 2024
@lmhs lmhs force-pushed the 5700-system-allow-to-establish-infinit-number-of-bonds-from-monomer-to-microstructure branch from ccf6c86 to a3b77a3 Compare January 24, 2025 14:55
@lmhs lmhs marked this pull request as ready for review January 24, 2025 15:21
@lmhs lmhs requested a review from tanas80 January 24, 2025 15:22
@lmhs lmhs force-pushed the 5700-system-allow-to-establish-infinit-number-of-bonds-from-monomer-to-microstructure branch 2 times, most recently from b13671d to 232ce4d Compare January 29, 2025 11:44
@@ -465,21 +466,36 @@ class PolymerBond implements BaseTool {

const atomRenderer = event.target.__data__ as AtomRenderer;
const monomer = this.bondRenderer?.polymerBond.firstMonomer;

if (
monomer instanceof Chem &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please delete monomer instanceof Chem && because restriction should work for all types of monomers

Copy link
Collaborator

Choose a reason for hiding this comment

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

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

I saw the description.
I'm 100% sure that it should work for every type of monomer. Discussed with @AlexeyGirin one more time, he approved. Please remove the line.

@rrodionov91 rrodionov91 merged commit 1e2a92a into master Feb 20, 2025
10 checks passed
@rrodionov91 rrodionov91 deleted the 5700-system-allow-to-establish-infinit-number-of-bonds-from-monomer-to-microstructure branch February 20, 2025 13:45
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.

System allow to establish infinit number of bonds from monomer to microstructure
4 participants