partridgejiang / Kekule.js

A Javascript cheminformatics toolkit.
http://partridgejiang.github.io/Kekule.js
MIT License
250 stars 61 forks source link

ElectronPushingArrow Issue or Maybe Feature Request #136

Open cleblond opened 5 years ago

cleblond commented 5 years ago

When an electron pushing arrow is started on a lone pair, the arrow object is an Arc in the object inspector and in the exported JSON. I think it should be an ElectronPushingArrow. In addition it would make sense if the arrow.getDonor() for such a case would point to the atom not the lone pair.

cleblond commented 5 years ago

It might be more flexible if arrow.getDonor() points to the lone pair object.

partridgejiang commented 5 years ago

Actually the end objects of the electron pushing arrow is currently restricted to node(atom) and connector(bond) objects only. In another word, the lone pair marker is not able to be "linked" with the arrow at all. Thanks a lot for your suggestion and such a feature may be implemented soon. At present, for a work-around, you may link the electron pushing arrow with the lone pair atom instead.

partridgejiang commented 5 years ago

Hi @cleblond, the electron pushing arrow is able to point to a lone pair now, please check the latest dist files. When pointing to a lone pair, arrow.getDonor() will still return the atom instance. To get the lone pair from the arrow, a new introduced method arrow.getDirectDonor() can be used.

cleblond commented 5 years ago

Thanks for the update. Looks like the electronpushingarrows can start from lone pairs now...Awesome!

Unfortunately when exported as Kekule-JSON (i.e. using Kekule.IO.saveFormatData(molecule, 'Kekule-JSON') it seems that the receptor position is incorrect.

For example if i construct this structure ss1

upon exporting I get this ss2

partridgejiang commented 5 years ago

-_-b, a silly bug introduced in the refactor of some indexing methods. Now it should be fixed, please check the latest dist files. @cleblond, thanks a lot again.

cleblond commented 5 years ago

No problem. Thanks you!

cleblond commented 5 years ago

Unfortunately after testing the new dist files, the curved arrows cannot be attached to atoms, bonds or lone pairs.

partridgejiang commented 5 years ago

Er... The arrow works fine in my test with the latest dist: image @cleblond, any error message has been reported in the console panel of your browser? Or would you please attach the exported Kekule-JSON file here? Thanks a lot again.

cleblond commented 5 years ago

There are no errors in the js console. I cleared my cache as usual and it doesn't allow arrows to attach. If I revert back to previous version arrows can be attached. Here is an animated gif. animated_gif

Here is the export kekule-json.

kekule-json.txt

cleblond commented 5 years ago

I did another pull and it seems OK now. Sorry for wasting your time. Thanks much!