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.

Sign up for GitHub

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

Jump to bottom

The reaction with catalysts is displayed incorrectly with ACS style setting and after layout #2458

Closed
AnnaKosol opened this issue Oct 1, 2024 · 5 comments · Fixed by #2560, #2561, #2637 or #2638
Closed

The reaction with catalysts is displayed incorrectly with ACS style setting and after layout #2458

AnnaKosol opened this issue Oct 1, 2024 · 5 comments · Fixed by #2560, #2561, #2637 or #2638
Assignees
@AliaksandrDziarkach
Labels
ACS standard Bug Layout Priority: High Team G
Milestone
Indigo-1.26.0-rc.3

Comments

@AnnaKosol
Copy link
Collaborator

AnnaKosol commented Oct 1, 2024 •
edited by AlexeyGirin
Loading

Ketcher part: epam/ketcher#5650

Steps to Reproduce

  1. add reaction to the Canvas
    error with cat and arr.ket.zip
Screenshot 2024-10-01 at 14 04 58
  1. click on ACS Style button in the Settings
  2. click on Apply button in the Settings
  3. click on layout

Ketcher part: epam/ketcher#5650

Actual behavior
Screenshot 2024-10-01 at 14 07 34

Expected behavior
All atoms are visible
Screenshot 2024-10-01 at 14 09 38

Environment details:

Issues for #2176
@AliaksandrDziarkach AliaksandrDziarkach added this to the Indigo-1.25.0-rc.2 milestone Oct 4, 2024
This was referenced Oct 7, 2024
Closed
Closed
AliaksandrDziarkach added a commit that referenced this issue Oct 16, 2024
@AliaksandrDziarkach
#2458 The reaction with catalysts is displayed incorrect with ACS st…
bb5e6f0 
…yle setting and after layout

Add code to calculate boundig box using atom labels.
Add UT
@AliaksandrDziarkach AliaksandrDziarkach linked a pull request Oct 16, 2024 that will close this issue
#2458 The reaction with catalysts is displayed incorrect with ACS style setting and after layout #2560
Merged
8 tasks
AliaksandrDziarkach added a commit that referenced this issue Oct 17, 2024
@AliaksandrDziarkach
Backmerge: #2458 The reaction with catalysts is displayed incorrect w…
878ea21 
…ith ACS style setting and after layout

backmerge to master
@AliaksandrDziarkach AliaksandrDziarkach linked a pull request Oct 17, 2024 that will close this issue
Backmerge: #2458 The reaction with catalysts is displayed incorrect with ACS style setting and after layout #2561
Merged
4 tasks
AlexanderSavelyev pushed a commit that referenced this issue Oct 17, 2024
@AliaksandrDziarkach
#2458 The reaction with catalysts is displayed incorrect with ACS sty…
13d2d36 
…le setting and after layout (#2560)
AlexanderSavelyev pushed a commit that referenced this issue Oct 17, 2024
@AliaksandrDziarkach
Backmerge: #2458 The reaction with catalysts is displayed incorrect w…
73d9e08 
…ith ACS style setting and after layout (#2561)
@Zhirnoff
Copy link
Collaborator

Should be tested after fix on Ketcher side in 2.26.0-rc.6 epam/ketcher#5650

@AlexeyGirin
Copy link
Collaborator

AlexeyGirin commented Oct 25, 2024 •
edited
Loading

Not fixed. Catalyst bounding boxes and margins are overlap each other. (they shouldn't)
image

  • Ketcher Version 2.26.0-rc.6 Build at 2024-10-25; 09:57:12
  • Indigo Toolkit Version 1.25.0-rc.4.0-g8862ccb7e-wasm32-wasm-clang-19.0.0
  • Chrome Version 130.0.6723.59 (Official Build) (64-bit)
  • Win10

@AlexeyGirin AlexeyGirin reopened this Oct 25, 2024
@AliaksandrDziarkach
Copy link
Collaborator

In preview all is ok:
image

So, this is ketcher issue.

@AlexeyGirin
Copy link
Collaborator

AlexeyGirin commented Nov 11, 2024 •
edited
Loading

Reopened.
Decision:
Current distance between atoms (above arrow) calculated based on average size of the letter.
It was decided to increase it slightly in order to increase distance between atoms (see on the pic below)
Follow up issue for ketcher created: epam/ketcher#5954
image

  • Ketcher Version 2.26.0-rc.9 Build at 2024-11-11; 12:01:35
  • Indigo Toolkit Version 1.25.0-rc.5.0-g45d28b8ca-wasm32-wasm-clang-19.0.0
  • Chrome Version 130.0.6723.117 (Official Build) (64-bit)
  • Win10

@AlexeyGirin AlexeyGirin reopened this Nov 11, 2024
@AlexeyGirin AlexeyGirin modified the milestones: Indigo-1.25.0-rc.5, Indigo-1.25.0-rc.6 Nov 11, 2024
@AlexeyGirin AlexeyGirin mentioned this issue Nov 11, 2024
Closed
@AlexeyGirin AlexeyGirin modified the milestones: Indigo-1.25.0-rc.6, Indigo-1.26.0-rc.3 Nov 15, 2024
@AlexeyGirin
Copy link
Collaborator

AlexeyGirin commented Nov 15, 2024 •
edited
Loading

Moved to Indigo-1.26.0-rc.3 as per decision made on meeting

@AlekseiKhariukov AlekseiKhariukov changed the title The reaction with catalysts is displayed incorrect with ACS style setting and after layout The reaction with catalysts is displayed incorrectly with ACS style setting and after layout Nov 15, 2024
AliaksandrDziarkach added a commit that referenced this issue Nov 15, 2024
@AliaksandrDziarkach
#2458 - The reaction with catalysts is displayed incorrectly with AC…
c582049 
…S style setting and after layout

Increase font bounding box. Update UT.
AliaksandrDziarkach added a commit that referenced this issue Nov 16, 2024
@AliaksandrDziarkach
Backmerge: #2458 - The reaction with catalysts is displayed incorrect…
9c8f4a9 
…ly with ACS style setting and after layout

backmerge to master
This was linked to pull requests Nov 16, 2024
#2458 - The reaction with catalysts is displayed incorrectly with ACS style setting and after layout #2637
Merged
Backmerge: #2458 - The reaction with catalysts is displayed incorrectly with ACS style setting and after layout #2638
Merged
AlexanderSavelyev pushed a commit that referenced this issue Nov 18, 2024
@AliaksandrDziarkach
#2458 - The reaction with catalysts is displayed incorrectly with AC…
82292ca 
…S style setting and after layout (#2637)

Co-authored-by: Aliakasndr Dziarkach <Aliakasndr.Dziarkach@gmail.com>
AlexanderSavelyev pushed a commit that referenced this issue Nov 18, 2024
@AliaksandrDziarkach
Backmerge: #2458 - The reaction with catalysts is displayed incorrect…
7c0b09c 
…ly with ACS style setting and after layout (#2638)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AliaksandrDziarkach AliaksandrDziarkach

Labels
ACS standard Bug Layout Priority: High Team G
Projects
None yet
Milestone
Indigo-1.26.0-rc.3
4 participants
@AliaksandrDziarkach @AlexeyGirin @Zhirnoff @AnnaKosol