diff --git a/.github/workflows/build-preview.yml b/.github/workflows/build-preview.yml index 16c259678..5a1fe803a 100644 --- a/.github/workflows/build-preview.yml +++ b/.github/workflows/build-preview.yml @@ -26,6 +26,11 @@ jobs: uses: actions/checkout@v3 with: ref: main + path: main + - name: Checkout PR Branch 🛎️ + uses: actions/checkout@v3 + with: + path: pr-branch - name: Install and Build Main Branch 🔧 run: | npm ci --include=dev @@ -33,16 +38,8 @@ jobs: npm run style npm run shields cp src/configs/config.aws.js src/config.js - - name: Capture main branch usage statistics - id: main-stats - run: | - MAIN_STATS=$(node scripts/stats.js -j) - echo "MAIN_STATS<> $GITHUB_ENV - echo -e "$MAIN_STATS" >> $GITHUB_ENV - echo "EOF" >> $GITHUB_ENV - - name: Checkout PR Branch 🛎️ - uses: actions/checkout@v3 - - name: Install and Build 🔧 + working-directory: main + - name: Install and Build PR Branch 🔧 # TODO: when we move shieldlib to its own repo, move shieldlib docs CI also run: | npm ci --include=dev @@ -52,11 +49,42 @@ jobs: cp src/configs/config.aws.js src/config.js mkdir -p dist/shield-docs cp -r shieldlib/docs/* dist/shield-docs - - name: Upload Build artifact - uses: actions/upload-artifact@v3 - with: - name: americana - path: dist/ + mv dist .. + working-directory: pr-branch + - name: Capture main branch usage statistics + id: main-stats + run: | + MAIN_STATS=$(node ../pr-branch/scripts/stats.js -j) + echo "MAIN_STATS<> $GITHUB_ENV + echo -e "$MAIN_STATS" >> $GITHUB_ENV + echo "EOF" >> $GITHUB_ENV + working-directory: main + - name: Start main branch map server + id: main-start-server + run: | + npm start & + until nc -z localhost 1776; do sleep 1; done + working-directory: main + - name: Capture main branch sample clips + # Run PR branch scripts against server running from main branch + id: main-samples + run: | + npm run generate_samples + mv samples ../samples-main + working-directory: pr-branch + - name: Stop main branch map server + id: main-stop-server + run: kill $(lsof -t -i:1776) 2>/dev/null || echo "No process found listening on port 1776." + working-directory: main + - name: Capture PR branch sample clips + id: pr-samples + run: | + npm start & + until nc -z localhost 1776; do sleep 1; done + npm run generate_samples + kill $(lsof -t -i:1776) 2>/dev/null || echo "No process found listening on port 1776." + mv samples ../samples-pr + working-directory: pr-branch - name: Capture PR branch usage statistics id: pr-stats run: | @@ -64,14 +92,29 @@ jobs: echo "PR_STATS<> $GITHUB_ENV echo -e "$PR_STATS" >> $GITHUB_ENV echo "EOF" >> $GITHUB_ENV + working-directory: pr-branch - name: Compare Stats id: compare-stats run: | - mkdir -p pr + mkdir -p ../pr echo '${{ env.MAIN_STATS }}' echo '${{ env.PR_STATS }}' - npm exec tsx scripts/stats_compare '${{ env.MAIN_STATS }}' '${{ env.PR_STATS }}' > pr/stats-difference.md - - name: Save PR artifacts + npm exec tsx scripts/stats_compare '${{ env.MAIN_STATS }}' '${{ env.PR_STATS }}' > ../pr/stats-difference.md + working-directory: pr-branch + - name: Generate map diff sample clips + id: map-diff-samples + run: | + npm exec tsx scripts/folder_diff ../samples-main ../samples-pr https://preview.ourmap.us/pr/${{ github.event.pull_request.number }}/ ${{ github.event.pull_request.head.sha }} + mv pr_preview-extra.md ../pr/ + cat ../pr/pr_preview-extra.md + mv samples-diff ../dist/ + working-directory: pr-branch + - name: Upload Build artifacts + uses: actions/upload-artifact@v3 + with: + name: americana + path: dist/ + - name: Save PR attributes env: PR_NUMBER: ${{ github.event.pull_request.number }} PR_SHA: ${{ github.event.pull_request.head.sha }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5415fba27..216ac2039 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -405,3 +405,7 @@ For consistency, POI icons use the following color palette: ## Fonts Fonts for style labels are packaged and defined in [fontstack66](https://github.com/osm-americana/fontstack66), Americana's font package. + +## Render Samples + +A GitHub action will check a list of regression test locations to see if the map has changed. If any of those locations have changed visually, the "Map Preview" check will generate before and after images. If your PR changes the visual appearance of the map, add an entry to `test/sample_locations.json` with a location that best illustrates the change. This will help show your change to PR reviewers as well as act as a regression test for future PRs. diff --git a/scripts/folder_diff.ts b/scripts/folder_diff.ts new file mode 100644 index 000000000..93263faf8 --- /dev/null +++ b/scripts/folder_diff.ts @@ -0,0 +1,91 @@ +import fs from "fs"; +import { basename } from "path"; +import { execSync } from "child_process"; + +interface Viewport { + width: number; + height: number; +} + +interface LocationData { + location: string; + name: string; + viewport: Viewport; + controls?: boolean; +} + +type Locations = LocationData[]; + +import sampleLocationJSON from "../test/sample_locations.json"; + +const sampleLocations: Locations = sampleLocationJSON as Locations; + +function getLocationByName(name: string): string | undefined { + const locationData = sampleLocations.find( + (location) => location.name === name + ); + return locationData?.location; +} + +// Check if the right number of arguments are passed +if (process.argv.length !== 6) { + console.log("Usage: "); + process.exit(1); +} + +const [folder1, folder2, urlBase, sha] = process.argv.slice(2); +const outputFolder = "samples-diff"; + +// Create the output folder if it doesn't exist +if (!fs.existsSync(outputFolder)) { + fs.mkdirSync(outputFolder); +} + +// Loop through files in folder1 +fs.readdirSync(folder1) + .filter((file) => file.endsWith(".png")) + .forEach((file) => { + const basefile = basename(file); + + // Check if file exists in folder2 + if (fs.existsSync(`${folder2}/${basefile}`)) { + // Compare the files + try { + execSync(`cmp -s "${folder1}/${basefile}" "${folder2}/${basefile}"`); + } catch (e) { + // If files are different + fs.copyFileSync( + `${folder1}/${basefile}`, + `${outputFolder}/${basefile.split(".")[0]}_${sha}_before.png` + ); + fs.copyFileSync( + `${folder2}/${basefile}`, + `${outputFolder}/${basefile.split(".")[0]}_${sha}_after.png` + ); + } + } + }); + +const outputMD = "pr_preview-extra.md"; +let mdContent = + "## Map Changes\n| Sample | Current Render | This PR |\n|-------------|--------|-------|\n"; + +// Loop through *_before.png files in the output folder +fs.readdirSync(outputFolder) + .filter((file) => file.endsWith("_before.png")) + .forEach((before_file) => { + const basefile = basename(before_file, `_${sha}_before.png`); + + // Check if the after file exists + if (fs.existsSync(`${outputFolder}/${basefile}_${sha}_after.png`)) { + // Add an entry to the markdown table + const loc = getLocationByName(basefile); + mdContent += + `| ${basefile}
${loc}
[Current Render](https://zelonewolf.github.io/openstreetmap-americana/#map=${loc})` + + `
[This PR](${urlBase}#map=${loc}) ` + + `| ![Current Render](${urlBase}${outputFolder}/${basefile}_${sha}_before.png) |` + + ` ![This PR](${urlBase}${outputFolder}/${basefile}_${sha}_after.png) |\n`; + } + }); + +fs.writeFileSync(outputMD, mdContent); diff --git a/src/layer/park.js b/src/layer/park.js index 3cb7f1b4c..662f3af01 100644 --- a/src/layer/park.js +++ b/src/layer/park.js @@ -3,23 +3,29 @@ import * as Label from "../constants/label.js"; import * as Color from "../constants/color.js"; +const parkLayerFilter = ["!=", ["get", "class"], "aboriginal_lands"]; + export const fill = { id: "protected-area_fill", + filter: parkLayerFilter, type: "fill", paint: { "fill-color": Color.parkFill, }, source: "openmaptiles", + minzoom: 5, "source-layer": "park", }; export const outline = { id: "protected-area_outline", + filter: parkLayerFilter, type: "line", paint: { "line-color": Color.parkOutline, }, source: "openmaptiles", + minzoom: 5, metadata: {}, "source-layer": "park", }; @@ -27,7 +33,7 @@ export const outline = { export const label = { id: "protected-area_label", type: "symbol", - filter: ["has", "rank"], + filter: ["all", ["has", "rank"], parkLayerFilter], paint: { "text-color": Color.parkLabel, "text-halo-blur": 1, diff --git a/test/sample_locations.json b/test/sample_locations.json index 689a07e94..9fc9c110a 100644 --- a/test/sample_locations.json +++ b/test/sample_locations.json @@ -12,16 +12,40 @@ "location": "13/40.01264/-75.70446", "name": "downingtown_alt_trk_bus", "viewport": { - "width": 600, - "height": 600 + "width": 400, + "height": 400 } }, { "location": "13/35.22439/-99.99495", "name": "historic_us66_oklahoma", "viewport": { - "width": 600, - "height": 600 + "width": 400, + "height": 400 + } + }, + { + "location": "15/33.92208/-83.37654", + "name": "athens_ga_7_way_concurrency", + "viewport": { + "width": 400, + "height": 400 + } + }, + { + "location": "16/40.765073/-73.965035", + "name": "manhattan_poi", + "viewport": { + "width": 400, + "height": 400 + } + }, + { + "location": "12/52.43818/-1.70563", + "name": "uk_all_roads_routes_z12", + "viewport": { + "width": 400, + "height": 400 } } ] diff --git a/tsconfig.json b/tsconfig.json index c4d433a91..d457a561d 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -3,6 +3,7 @@ "target": "ESNext", "module": "ESNext", "moduleResolution": "node", + "resolveJsonModule": true, "allowSyntheticDefaultImports": true }, "include": ["src/**/*.ts", "shieldlib/src/headless_graphics.ts"]