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

Settings: 'Terminal and Hetero' is not selected as default and 'on' option is not working #2245

Closed
Zhirnoff opened this issue Feb 20, 2023 · 3 comments · Fixed by #2477 or #2531
Closed
Assignees
Labels

Comments

@Zhirnoff
Copy link
Collaborator

Steps to Reproduce:

Scenario 1 ('Terminal and Hetero' is not set for default)

  1. Open Ketcher
  2. Select: Settings->Atoms-> Show hydrogen labels
  3. Look at selected option

Actual behavior
There is selected 'on' as default

Expected behavior
There is selected 'Terminal and Hetero' as default

Screenshots
2023-02-20_16h03_24
2023-02-20_16h03_37

Scenario 2 (The 'on' option is not working as expected):

  1. Open Ketcher
  2. Add 'Benzene' on canvas
  3. Select: Settings->Atoms-> Show hydrogen labels
  4. From drop-down select 'on' and press 'Apply'
  5. Check appearence hydrogen labels on the molecules.

Actual behavior
The hydrogen labels do not appear on the molecules after turning 'on' Show hydrogen labels option.

Expected behavior
The hydrogen labels should appear on the molecules after turning 'on' Show hydrogen labels option.
2023-02-20_16h19_43

Screenshots
2023-02-20_16h20_13

Scenario 3 (Special nodes 'Deuterium', 'Tritium'):
Precondition: 'Terminal and Hetero' is set for default in Settings->Atoms-> Show hydrogen labels

  1. Open Ketcher
  2. Select: Extended table->Special nodes-> put 'D' and 'T' on canvas
  3. Check appearence hydrogen labels on 'D' and 'T'.

Actual behavior
The hydrogen labels do not appear on 'D' and 'T'.

Expected behavior
The hydrogen labels should appear on 'D' and 'T'.
2023-02-20_16h40_21

Screenshots
2023-02-20_16h39_57

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser Chrome
  • Version 109.0.5414.120 (Official Build) (64-bit)

Ketcher version
Version 2.8.0-rc.3

@Zhirnoff Zhirnoff added the bug label Feb 20, 2023
@Zhirnoff Zhirnoff added this to the Ketcher 2.9.0-rc.1 milestone Feb 20, 2023
@Zhirnoff Zhirnoff changed the title Settings: 'Show hydrogen labels' issues Settings: 'Terminal and Hetero' is not selected as default and 'on' option is not working Apr 5, 2023
@Nitvex Nitvex assigned yuleicul and unassigned Nitvex Apr 10, 2023
@yuleicul
Copy link
Collaborator

yuleicul commented Apr 11, 2023

Scenario 3 (Special nodes 'Deuterium', 'Tritium'):
Precondition: 'Terminal and Hetero' is set for default in Settings->Atoms-> Show hydrogen labels

Open Ketcher
Select: Extended table->Special nodes-> put 'D' and 'T' on canvas
Check appearence hydrogen labels on 'D' and 'T'.
Actual behavior
The hydrogen labels do not appear on 'D' and 'T'.

Expected behavior
The hydrogen labels should appear on 'D' and 'T'.
2023-02-20_16h40_21

[Answered: D or T requires implicit H when it is a free form. Just like adding a H to the canvas, we also add an extra H to make it H2] Sorry, I don't understand here. From my understanding, Deuterium or Tritium is essentially hydrogen, why are we going to add an extra 'H' beside it?

yuleicul added a commit that referenced this issue Apr 12, 2023
yuleicul added a commit that referenced this issue Apr 12, 2023
KonstantinEpam23 pushed a commit that referenced this issue Apr 20, 2023
…`on` option is not working (#2477)

* #2245 - Fix `Show hydrogen labels` to make `Terminal and Hetero` default

* #2245 - Fix `on` option to show all labels

* #2245 - Fix D and T to have implicit H
@Zhirnoff
Copy link
Collaborator Author

The third scenario does not work as described in the issue when 'Terminal and Hetero' is set for default in Settings->Atoms-> Show hydrogen labels

2023-04-25_22h36_49

@Zhirnoff Zhirnoff reopened this Apr 25, 2023
KonstantinEpam23 added a commit that referenced this issue Apr 25, 2023
@KonstantinEpam23
Copy link
Collaborator

KonstantinEpam23 commented Apr 25, 2023

@Zhirnoff @yuleicul this was broken in #1865
Here is a fix: #2531

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment