-
Notifications
You must be signed in to change notification settings - Fork 25
Description
I'm writing a Rust library to render and search sigils, and I ran into a few difficulties related to not going through sigil-js. Some of them could easily be made better, and some are trickier or would involve tradeoffs that might not be acceptable. I was thinking about making a bunch of separate issues, but they feel related, so I thought it would be better to write them all up in one issue.
I haven't used JavaScript in a while, and I've never used TypeScript, so sorry for just complaining about all this stuff and not opening PRs.
Make canonicity of src/index.json obvious
When I started working on sigil rendering, I wasn't sure what the canonical source of sigil geometry was. Searching around, it seems like src/index.json is the canonical source of geometry. (Please correct me if I'm wrong! I'm not sure if there's another source around that I missed.)
If that's the case, it might be a good idea to make it more obvious that it's the canonical source of sigil geometry, so it's easier to find for users outside of sigil-js.
I would suggest moving src/index.json to sigils.json in the root of the project, as well as mentioning it prominently in the readme.
Replace @FG and @BG with colors
Currently, color attribute values in src/index.json are @FG or @BG, which aren't valid SVG colors. Downstream users will need to notice that they're not valid colors, and replace them when converting to SVG.
I would suggest replacing @FG and @BG with white and black. This would remove one post-processing step, and since the values white and black don't appear in other attributes, they can still be replaced to re-color the SVGs.
Is stroke-width correct?
When rendering the converted SVGs, i noticed that some of the strokes seemed to be incorrect. Here's a rendering of bac:
I'm not sure, but I think the stroke-width attribute values in src/index.json aren't correct. When rendering with sigils-js, stroke-width values are always overwritten, and so the values in index.json are never used.
I would suggest replacing the stroke-width value such that they render correctly without modification.
Remove dataisgeon attribute
I think that the dataisgeon attribute is only used by sigil-js, and not actually propagated to SVG. Would it be possible to remove this from src/index.json, perhaps by computing when it's necessary instead? This would remove an additional modification step that downstream users would otherwise have to perform.
Add vector-effect=non-scaling-stroke
This is another modification that downstream users need to make in order to get rendering correct. Adding it to the source data would make this easier.
Consider making SVG files the canonical source of sigil geometry
Currently, downstream users have to convert index.json into SVG, which isn't straightforward. The SVG -> JSON conversion isn't standard, and attributes have to be modified, added, or removed in order to produce valid SVG that renders correctly.
In order to avoid increasing the size of sigil-js, and avoiding making sigil-js do the SVG -> JSON conversion itself, my suggestion would be:
- Create a separate repository, e.g.
urbit/sigil, to hold the canonical sigil geometry data - Put each sigil in a separate SVG file, e.g.
data/${SIGIL_NAME}.svg - Make each sigil SVG file valid SVG that renders correctly without needing further modification
- Convert the sigil data from individual SVG files to a single JSON file for use in
sigil-js. Something nice here is that since this data would no longer be canonical, it could be minified to reduce the file size, makingsigil-jssmaller. Also, it could be JS instead of JSON, which would further reduce the file size, and avoid needing to do the conversion from JSON to actual JS objects in the library
