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

Fix minimal distances for mult-tail validation in KET format #5761

Closed
AlexanderSavelyev opened this issue Oct 15, 2024 · 3 comments · Fixed by #5771 or #5779
Closed

Fix minimal distances for mult-tail validation in KET format #5761

AlexanderSavelyev opened this issue Oct 15, 2024 · 3 comments · Fixed by #5771 or #5779
Assignees
Labels
MMPK Scope for Pathway reactions, View-only mode, Static images, Custom buttons and Ketcher API changes Multi-Tailed Arrow Tool Priority: High

Comments

@AlexanderSavelyev
Copy link
Contributor

AlexanderSavelyev commented Oct 15, 2024

Steps to Reproduce

  1. Insert structure from smiles "CC"
  2. Insert default multi-tail arrow
  3. Increase the bond length twice
  4. Select all
  5. Save as Ket, copy the ket file into clipboard buffer
  6. Paste the ket file

Actual behavior
Error message appears - cannot deserialize input JSON

Expected behavior
No errors, Structure is pasted but with a smaller size
The following validation rules should be changed for KET file (only for KET, the UI behavior should not be changed)

MIN_TAIL_DISTANCE - 0.01.
MIN_HEAD_LENGTH - 0.01.
MIN_TAIL_LENGTH - 0.01
MIN_TOP_BOTTOM_OFFSET - 0.01
MIN_HEIGHT - 0.01

Desktop (please complete the following information):

  • Version 2.26rc2

Note

@AlexanderSavelyev AlexanderSavelyev added this to the Ketcher 2.26.0-rc.3 milestone Oct 15, 2024
@ivanmaslow ivanmaslow added MMPK Scope for Pathway reactions, View-only mode, Static images, Custom buttons and Ketcher API changes Multi-Tailed Arrow Tool labels Oct 15, 2024
@daniil-sloboda daniil-sloboda linked a pull request Oct 16, 2024 that will close this issue
9 tasks
rrodionov91 pushed a commit that referenced this issue Oct 16, 2024
- updated minimal ket validation distance
- commented tests with minimal values because they are no longer valid
daniil-sloboda added a commit that referenced this issue Oct 16, 2024
- updated minimal ket validation distance
- commented tests with minimal values because they are no longer valid
@rrodionov91 rrodionov91 linked a pull request Oct 16, 2024 that will close this issue
9 tasks
@ivanmaslow
Copy link
Collaborator

ivanmaslow commented Oct 16, 2024

12 manual tests and files are updated, minimal sizes are retested on UI and for loading from KET on Nightly env.

  • Minimal sizes on UI are still working as it was:
    • Minimal length of spine without tails = 0.5:
      image
    • Minimal distance between tails = 0.35
    • Minimal length of tail = 0.4
    • Minimal length of head = 0.5
    • Minimal X distance between head and top/bottom tails = 0.15:
      image
  • As for minimal sizes for loading from KET, they equal 0.01, but there is no chance to verify validation for MIN_HEIGHT - 0.01 as we have the same MIN_TOP_BOTTOM_OFFSET - 0.01, this is part of MIN_HEIGHT.
    • After loading of Multi-Tailed arrow with minimal sizes, it looks like that, but it can be adjusted to normal size using resizing of head, tails and spine:
      image
      image
  • One defect is detected related to changes of minimal sizes - Uncaught TypeError is in console when trying to move tail of Multi-Tailed Arrow added from KET with small size between tails #5786

Guch1g0v pushed a commit that referenced this issue Oct 17, 2024
- updated minimal ket validation distance
- commented tests with minimal values because they are no longer valid
@rrodionov91
Copy link
Collaborator

Need merge into 2.26

@rrodionov91 rrodionov91 reopened this Oct 18, 2024
rrodionov91 pushed a commit that referenced this issue Oct 21, 2024
- updated minimal ket validation distance
- commented tests with minimal values because they are no longer valid
rrodionov91 pushed a commit that referenced this issue Oct 22, 2024
- updated minimal ket validation distance
- commented tests with minimal values because they are no longer valid
@Zhirnoff
Copy link
Collaborator

Tested. Bug fixed.
2024-10-25_15h11_36
2024-10-25_15h11_51
2024-10-25_15h12_25

2024-10-25_15h13_32.mp4

Desktop:

  • OS: Windows 11
  • Browser Chrome
  • Version 128.0.6613.120 (Official Build) (64-bit)

Ketcher version
[Version 2.26.0-rc.6]
Indigo version
[Version 1.25.0-rc.4]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MMPK Scope for Pathway reactions, View-only mode, Static images, Custom buttons and Ketcher API changes Multi-Tailed Arrow Tool Priority: High
Projects
None yet
7 participants