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

ignoreChiralFlag setting is not regarded for Templates #3019

Closed
Nitvex opened this issue Aug 2, 2023 · 4 comments · Fixed by #3087
Closed

ignoreChiralFlag setting is not regarded for Templates #3019

Nitvex opened this issue Aug 2, 2023 · 4 comments · Fixed by #3087

Comments

@Nitvex
Copy link
Collaborator

Nitvex commented Aug 2, 2023

Steps to Reproduce

  1. set ignore chiral flag option in settings to true
  2. Open Templates modal
  3. Choose any template without chiral flag (it is set to zero)
  4. Add it to canvas

Actual behavior
"And Enantiomer" label is added
image

Expected behavior
"And Enantiomer" label is not shown
image

Test case
Add test case number (in EPMLSOPKET-* format) if bug was found during test case run.

Additional context
This flag IS regarded, when pasting from file (0 chiral flag in MOLv2000 is set to 1), but this doesn't happen, when pasting from Templates

Note: issue is not reproducible with current set of structures.

Related issue: #3022

@Nitvex Nitvex added the bug label Aug 2, 2023
@Nitvex Nitvex added this to the Ketcher 2.14.0-rc.1 milestone Aug 2, 2023
@Nitvex Nitvex self-assigned this Aug 2, 2023
@Nitvex Nitvex changed the title ignoreChiralFlag option is not regarded for Templates ignoreChiralFlag setting is not regarded for Templates Aug 2, 2023
@nanoblit nanoblit assigned nanoblit and unassigned Nitvex Aug 9, 2023
@Zhirnoff
Copy link
Collaborator

Zhirnoff commented Aug 9, 2023

Manual test cases will be written by me

@Nitvex
Copy link
Collaborator Author

Nitvex commented Aug 10, 2023

Additional clarifications:

  1. If chiral flag in file is set to "1", then DO NOT show the label.
  2. If chiral flag in file is set to "0", then DO show the label.
  3. If ignore chiral flag setting is enabled, then consider the chiral flag to be equal to "1"
  4. If ignore chiral flag setting is disabled, then left chiral flag as is.

@nanoblit
Copy link
Collaborator

nanoblit commented Aug 11, 2023

Further clarification:

  • In case that multiple different types of stereo labels appear in the structure - the stereo labels are displayed, but the overall label is not displayed.
  • In case that at most one type of stereo labels appears in the structure - the stereo labels are not displayed, but the overall label is displayed.

Rules in the previous comment apply for both stereo labels and the overall label.

@nanoblit
Copy link
Collaborator

nanoblit commented Aug 23, 2023

What is wrong is that the templates are loaded without options and then aren't reloaded when the options change, so ignoreChiralFlag is always "undefined". That's what needs to be fixed.

nanoblit added a commit that referenced this issue Aug 28, 2023
nanoblit added a commit that referenced this issue Aug 29, 2023
nanoblit added a commit that referenced this issue Aug 29, 2023
nanoblit added a commit that referenced this issue Aug 31, 2023
Nitvex pushed a commit that referenced this issue Aug 31, 2023
* #3019 - Show stereo label correctly

* #3019 - Rerender template previews after options have changed

* #3019 - Simplify template render invalidation code

* #3019 - Initial changes to logic for showing labels

* #3019 - Fix templates not being updated after changing ignoreChiralFlag setting

* #3019 - Fix MolSerializer error

* #3019 - Remove unused variable

* #3019 - Fix localStorage usage

* #3019 - Add Phenylalanice mustard to template library

* #3019 - Add tests

* #3019 - Fix tests

* #3019 - Fix tests

* #3019 - Add types to init-lib and explanation to tests

* #3019 - Make a test more readable
@MartaWilliams MartaWilliams self-assigned this Sep 6, 2023
@github-project-automation github-project-automation bot moved this to Done in Ketcher Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants