diff --git a/.github/workflows/generate-sprites.yml b/.github/workflows/generate-sprites.yml index a6ce9b4..8327dc2 100644 --- a/.github/workflows/generate-sprites.yml +++ b/.github/workflows/generate-sprites.yml @@ -4,6 +4,7 @@ on: pull_request: paths: - "sprite_assets/**" + - "scripts/**" concurrency: group: sprites-${{ github.event.pull_request.number }} @@ -22,6 +23,9 @@ jobs: ref: ${{ github.event.pull_request.head.ref }} fetch-depth: 0 + - name: Run script tests + run: python3 -m unittest discover scripts + - name: Detect changed namespaces id: namespaces run: | diff --git a/.gitignore b/.gitignore index a497361..bad57e7 100755 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,10 @@ # .DS_Store +# generate_sprites.sh's intermediate output (see scripts/strip_stretchable_markers.py) +.sprite_build/ + +# Python +__pycache__/ +*.pyc + diff --git a/README.md b/README.md index 920bf93..5f58eed 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,8 @@ ## ๐Ÿงพ Instructions 0. **Pre-requisites** - Docker must be installed. This is used to run a martin tile server image, which is the one that converts source files to sprites. + - Docker must be installed. This is used to run a martin tile server image, which is the one that converts source files to sprites. + - Python 3 must be installed. `generate_sprites.sh` uses it to strip the stretchable-icon marker fills (`content`/`stretchX`/`stretchY`) out of the SVGs before martin sees them (see `scripts/strip_stretchable_markers.py`). No extra packages needed - it only uses the standard library. 1. **Make sure that the assets in `sprite_assets` are correct.** It is essential that the file names are correct. @@ -19,7 +20,12 @@ bash generate_sprites.sh AtB FRAM ``` -> **CI:** When a pull request touches files under `sprite_assets/`, GitHub Actions automatically regenerates sprites for the changed namespaces and commits the result back to the PR branch. No manual script run needed. +> **CI:** When a pull request touches files under `sprite_assets/` or `scripts/`, GitHub Actions runs the script tests, then (if `sprite_assets/` changed) regenerates sprites for the changed namespaces and commits the result back to the PR branch. No manual script run needed. + + **Running the script tests locally:** + ```sh + python3 -m unittest discover scripts + ``` 3. **Upload sprites to GCS** diff --git a/generate_sprites.sh b/generate_sprites.sh index dfadcfe..a8c36ed 100644 --- a/generate_sprites.sh +++ b/generate_sprites.sh @@ -2,9 +2,16 @@ set -e # Exit on error +# Pre-requisite check +command -v python3 >/dev/null 2>&1 || { + echo "โŒ python3 is required (used by scripts/strip_stretchable_markers.py) but was not found on PATH." >&2 + exit 1 +} + # Config BASE_URL="http://localhost:3000/sprite" OUT_BASE="./generated_sprites" +BUILD_DIR="./.sprite_build" THEMES=("light" "dark") RESOLUTIONS=("" "@2x") @@ -26,7 +33,14 @@ echo "๐Ÿ”„ Stopping and removing any existing 'martin' container..." docker stop martin >/dev/null 2>&1 || true docker rm martin >/dev/null 2>&1 || true -# 2. Build volume mounts and --sprite args +# 2. Strip stretch/content marker fills, then build volume mounts and --sprite args +# (SVGs in sprite_assets/ carry visible marker rects - e.g. id="mapbox-content" - +# so Figma doesn't drop them as invisible shapes on export. They need to be made +# invisible before martin rasterises them, without touching the geometry martin +# reads their content/stretchX/stretchY from. See scripts/strip_stretchable_markers.py) +echo "๐Ÿฉน Stripping stretch/content marker fills..." +rm -rf "$BUILD_DIR" + echo "๐Ÿ“ฆ Building volume mounts and sprite args..." VOLUME_ARGS=() SPRITE_ARGS=() @@ -34,10 +48,12 @@ SPRITE_ARGS=() for NAME in "${NAMESPACES[@]}"; do for THEME in "${THEMES[@]}"; do SRC_DIR="$(pwd)/sprite_assets/${NAME}/${THEME}" - TARGET_DIR="/sprite_assets/${NAME}_${THEME}" + PROCESSED_DIR="$(pwd)/${BUILD_DIR}/${NAME}_${THEME}" + mkdir -p "$PROCESSED_DIR" + python3 scripts/strip_stretchable_markers.py "$SRC_DIR" "$PROCESSED_DIR" - - VOLUME_ARGS+=(-v "${SRC_DIR}:${TARGET_DIR}") + TARGET_DIR="/sprite_assets/${NAME}_${THEME}" + VOLUME_ARGS+=(-v "${PROCESSED_DIR}:${TARGET_DIR}") SPRITE_ARGS+=(--sprite "${TARGET_DIR}") done done diff --git a/scripts/strip_stretchable_markers.py b/scripts/strip_stretchable_markers.py new file mode 100755 index 0000000..6a5b13d --- /dev/null +++ b/scripts/strip_stretchable_markers.py @@ -0,0 +1,118 @@ +#!/usr/bin/env python3 +""" +Strips the paint (fill/stroke) from Mapbox stretchable-icon marker elements +in exported SVGs, while leaving their id and geometry untouched. + +Figma drops fully-transparent shapes when exporting to SVG, so the marker +rects for `content`/`stretchX`/`stretchY` (see +https://docs.mapbox.com/style-spec/reference/sprite/) have to be exported +with a visible, solid-ish fill and then made invisible afterwards - that's +what this script does. martin/spreet only look at each marker's bounding box +via its `id`, so removing the paint has no effect on the generated sprite +metadata. + +Usage: + strip_stretchable_markers.py + +Every .svg file under is processed into the same relative path +under . Files with no marker elements are copied through unchanged. +""" + +import re +import shutil +import sys +import xml.etree.ElementTree as ET +from pathlib import Path + +SVG_NS = "http://www.w3.org/2000/svg" +ET.register_namespace("", SVG_NS) + +# Matches the exact ids martin/spreet look for: +# mapbox-content, mapbox-stretch, mapbox-stretch-x[-N], mapbox-stretch-y[-N] +MARKER_ID_RE = re.compile(r"^mapbox-(content|stretch(-[xy](-\d+)?)?)$") + +# Figma appends `_2`, `_3`, ... to de-duplicate layers with the same name, +# which silently breaks the convention above (spreet will never find e.g. +# "mapbox-content_2"). Flag these so they can be renamed by hand in Figma. +SUSPECT_ID_RE = re.compile(r"^mapbox-(content|stretch(-[xy])?)(-\d+)?_\d+$") + +PAINT_ATTRS = ("fill", "fill-opacity", "stroke", "stroke-width", "stroke-opacity") + + +def strip_paint(elem: ET.Element) -> None: + for attr in PAINT_ATTRS: + elem.attrib.pop(attr, None) + elem.attrib.pop("style", None) + elem.set("fill", "none") + elem.set("stroke", "none") + + +def process_svg(src_path: Path) -> tuple[bytes, list[str]]: + warnings = [] + tree = ET.parse(src_path) + root = tree.getroot() + + seen_marker_ids = set() + touched = False + for elem in root.iter(): + elem_id = elem.get("id") + if not elem_id: + continue + if MARKER_ID_RE.match(elem_id): + if elem_id in seen_marker_ids: + warnings.append( + f"duplicate marker id {elem_id!r} - only one will be used by spreet" + ) + seen_marker_ids.add(elem_id) + strip_paint(elem) + touched = True + elif SUSPECT_ID_RE.match(elem_id): + warnings.append( + f"id {elem_id!r} looks like a Figma auto-deduplicated marker " + "name (e.g. two layers were both named 'mapbox-content') - " + "rename it in Figma so it matches the plain mapbox-* convention" + ) + + if not touched: + return src_path.read_bytes(), warnings + + return ET.tostring(root, encoding="unicode").encode("utf-8"), warnings + + +def main() -> int: + if len(sys.argv) != 3: + print(__doc__, file=sys.stderr) + return 1 + + src_dir, dst_dir = Path(sys.argv[1]), Path(sys.argv[2]) + if not src_dir.is_dir(): + print(f"error: {src_dir} is not a directory", file=sys.stderr) + return 1 + + dst_dir.mkdir(parents=True, exist_ok=True) + exit_code = 0 + for src_path in sorted(src_dir.glob("*.svg")): + dst_path = dst_dir / src_path.name + try: + content, warnings = process_svg(src_path) + except ET.ParseError as e: + print(f"error: failed to parse {src_path}: {e}", file=sys.stderr) + exit_code = 1 + continue + + dst_path.write_bytes(content) + for warning in warnings: + print(f"warning: {src_path.name}: {warning}", file=sys.stderr) + exit_code = 1 + + # carry over any non-svg files (shouldn't normally be any, but avoids + # silently dropping something martin would otherwise have seen) + for other_path in src_dir.iterdir(): + if other_path.is_file() and other_path.suffix.lower() != ".svg": + shutil.copy2(other_path, dst_dir / other_path.name) + + return exit_code + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/test_strip_stretchable_markers.py b/scripts/test_strip_stretchable_markers.py new file mode 100644 index 0000000..dc0099b --- /dev/null +++ b/scripts/test_strip_stretchable_markers.py @@ -0,0 +1,98 @@ +#!/usr/bin/env python3 +"""Tests for strip_stretchable_markers.py. Run with: python3 -m unittest discover scripts""" + +import sys +import tempfile +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from strip_stretchable_markers import process_svg + +SVG_OPEN = '' +SVG_CLOSE = "" + + +class ProcessSvgTests(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + + def write_svg(self, body: str) -> Path: + path = Path(self.tmp.name) / "test.svg" + path.write_text(SVG_OPEN + body + SVG_CLOSE) + return path + + def test_marker_fill_is_stripped(self): + path = self.write_svg( + '' + ) + content, warnings = process_svg(path) + svg = content.decode() + + self.assertEqual(warnings, []) + self.assertIn('id="mapbox-content"', svg) + self.assertIn('fill="none"', svg) + self.assertIn('stroke="none"', svg) + # geometry must be untouched + for attr in ('x="1"', 'y="2"', 'width="3"', 'height="4"'): + self.assertIn(attr, svg) + self.assertNotIn("#FF00FF", svg) + self.assertNotIn("fill-opacity", svg) + + def test_numbered_stretch_ids_are_stripped(self): + path = self.write_svg( + '' + '' + ) + content, warnings = process_svg(path) + svg = content.decode() + + self.assertEqual(warnings, []) + self.assertEqual(svg.count('fill="none"'), 2) + + def test_non_marker_ids_are_untouched(self): + path = self.write_svg( + '' + ) + content, warnings = process_svg(path) + svg = content.decode() + + self.assertEqual(warnings, []) + self.assertIn('fill="white"', svg) + + def test_file_without_markers_is_byte_identical(self): + path = self.write_svg('') + original = path.read_bytes() + + content, warnings = process_svg(path) + + self.assertEqual(warnings, []) + self.assertEqual(content, original) + + def test_duplicate_marker_id_warns(self): + path = self.write_svg( + '' + '' + ) + _, warnings = process_svg(path) + + self.assertEqual(len(warnings), 1) + self.assertIn("duplicate marker id", warnings[0]) + + def test_figma_dedup_suffix_is_flagged_not_stripped(self): + path = self.write_svg( + '' + ) + content, warnings = process_svg(path) + svg = content.decode() + + self.assertEqual(len(warnings), 1) + self.assertIn("mapbox-content_2", warnings[0]) + # not a recognized marker id, so it must be left alone + self.assertIn('fill="red"', svg) + + +if __name__ == "__main__": + unittest.main()