reymond-group / smilesDrawer

A small, highly performant JavaScript component for parsing and drawing SMILES strings. Released under the MIT license.
https://smilesdrawer.rocks
MIT License
434 stars 68 forks source link

viewBox Fixes #134

Open swamidass opened 2 years ago

swamidass commented 2 years ago

Today I used patch-package to patch smiles-drawer@1.2.0 for the project I'm working on.

The issue this solves is two fold:

  1. that no serverside DOM has viewBox.baseVal
  2. the code should not enforce 1:1 aspect ratio, but set the viewBox to the intrinsic shape of the drawing.

Fixing the first problem allows serverside rendering. Fixing the second problem allows downstream code/css to determine the aspect ratio based on the intrinsic shape of the drawing.

This is all the setup needed to serverside render an SVG:

import {parseHTML} from 'linkedom'; var {document, window} = parseHTML(``); globalThis.document = document

Now, this simple code can produce an SVG (without polluting the global space):

` function draw(smiles, options = {}) { var smilesDrawer = new SmilesDrawer.Drawer(options); var svgDrawer = new SmilesDrawer.SvgDrawer(options);

return new Promise((resolve, reject) => {  
    SmilesDrawer.parse(smiles, resolve, reject)
})
.then((tree) =>  {
   let svg = document.createElement("svg")
   svgDrawer.draw(tree, svg, 'light', false)
   return svg
   }
)

}

console.log((await draw("CCCO")).outerHTML) ` This doesn't pollute the global namespace because smilesDrawer only uses document for "createElement" and to locate the target element. By providing the target element directly, instead of by ID string, there is no global polution.

Here is the diff that solved my problem:

diff --git a/node_modules/smiles-drawer/src/SvgWrapper.js b/node_modules/smiles-drawer/src/SvgWrapper.js
index adbc611..f9659fd 100644
--- a/node_modules/smiles-drawer/src/SvgWrapper.js
+++ b/node_modules/smiles-drawer/src/SvgWrapper.js
@@ -208,8 +208,8 @@ class SvgWrapper {
     this.drawingWidth = maxX - minX;
     this.drawingHeight = maxY - minY;

-    let w = this.svg.clientWidth > 0 ? this.svg.clientWidth : this.svg.viewBox.baseVal.width;
-    let h = this.svg.clientHeight > 0 ? this.svg.clientHeight : this.svg.viewBox.baseVal.height;
+    let w = this.drawingWidth
+    let h = this.drawingHeight

     let scaleX = w / this.drawingWidth;
     let scaleY = h / this.drawingHeight;
@@ -217,7 +217,7 @@ class SvgWrapper {
     let scale = (scaleX < scaleY) ? scaleX : scaleY;
     let viewBoxDim = Math.round(this.drawingWidth > this.drawingHeight ? this.drawingWidth : this.drawingHeight);  

-    this.svg.setAttributeNS(null, 'viewBox', `0 0 ${viewBoxDim} ${viewBoxDim}`);
+    this.svg.setAttributeNS(null, 'viewBox', `0 0 ${Math.round(w)} ${Math.round(h)}`);

     this.offsetX = -minX;
     this.offsetY = -minY;

This issue body was partially generated by patch-package.