Skip to content

Functionality Fixes October 2026 - #243

Closed
johnmay wants to merge 44 commits into
masterfrom
fixes-oct-2026
Closed

johnmay wants to merge 44 commits into
masterfrom
fixes-oct-2026

Conversation

@johnmay

@johnmay johnmay commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Important! Depends on #242
Currently checking test status on pipeline.

Changes:

  • Fix a glitch in the select module
  • Render the whole model in a single container
  • Fix a bug w.r.t to the hydrogen expand/contract action (with test added)
  • Ensure stereochemistry is not lost on "copy"
  • Allow multiple shortcuts for menu items and move some hot keys to the main event/action map
  • Change the "eraser"/"delete" behaviour, there is now a flag (all, default=true). When this flag is not set it "deletes" hetero atoms / charged carbons only back to normal carbons. This sounds odd but works quite nicely. (tests added)
  • Allow "eraser" to be an active draw module again but only if a user clicks the button not if they have pressed the hot key. Other modules (charge/symbol) were updated as well.
  • Add in "valence cycling", if a user sets the symbol of an atom which is already that symbol we now "cycle" the valence down CH4 > CH3 > CH2 etc. R atoms go R1, R2, R3. This is handled by the atom.getValency() property.
  • Improve the merge semantics allow hetero atoms to take priority and adding an "alt" mode which disables merging (tests added).
Screenshot 2026-10-14 at 07 35 42

Then merge:

Screenshot 2026-10-14 at 07 35 48

The nitrogens are 'kept'.

…All the menu tests are now together and all open/save tests are together.
There is possibly a better long term solution where we keep the stereochemistry from the original object without reperceiving it but this functions for now.
We can only have a single accelerator but we can have multiple keystokes mapped directly to the actions, however the mi.getAction() is null and we can't set it so we need pass it in.
We now only remove carbons, hetero atoms are only removed if all their bonds are removed otherwise they are changed to carbons. This sounds strange but works really nicely and mirrors some other chemical sketchers.
…s fired. Otherwise it is just the key which is pressed (not correct).
… element.

Setting the explicit valence clears any charge/isotope and likewise setting the charge clears the valence.
Set the default display mode to no terminal -CH3, no longer needed since the user can control it.
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
TestsPassed ✅SkippedFailed
JUnit Test Report99 ran99 passed0 skipped0 failed

@johnmay
johnmay marked this pull request as ready for review October 14, 2026 13:11
@johnmay johnmay changed the title WIP: Functionality Fixes October 2026 Functionality Fixes October 2026 Oct 14, 2026
@johnmay

johnmay commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

@egonw need #242 first but this is good for review now. Sorry some bits where I was fixing tests are a bit messy but summary above explains the main functionality.

When cdk/cdk#1338 is merge there is an option to enable as well.

@egonw

egonw commented Oct 17, 2026

Copy link
Copy Markdown
Member

Merged.

@egonw egonw closed this Oct 17, 2026
@johnmay
johnmay deleted the fixes-oct-2026 branch October 17, 2026 18:53
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.

2 participants