Skip to content

Gallery conversion for Sandcastle v2 #12752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: sandcastle-v2
Choose a base branch
from

Conversation

jjspace
Copy link
Contributor

@jjspace jjspace commented Jul 21, 2025

Description

This is inherently a big PR. We have over 250 sandcastle examples and in the new format they are each 4 files. To help facilitate review I've intentionally broken the change into 3 commits

  1. Creating the conversion script itself
  2. Running the conversion directly and ignoring any errors
  3. Fixing all eslint issues in the sandcastles

This should hopefully make reviewing easier but it also should help with running the script again later if we need to. I know there's a chance new sandcastles get created or updated outside this effort. We will do a final content pass later to give them all a final cleanup but for now it should be mostly automated. I'm leaving the script in the repo for now but plan to remove it as well once we can make the switch.

Issue number and link

Part of #12566

Testing plan

  • either run npm run dev from the sandcastle package or npm run build-sandcastle and npm start from the repo root
  • Scroll through the gallery and make sure all items show up (UI changed drastically in Sandcastle using Stratakit #12731)
  • Click on a handful of gallery items and make sure they work. lmk if you find broken ones but we will do a full pass on them all later.

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have updated the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code

@jjspace jjspace requested a review from ggetz July 21, 2025 14:57
Copy link

Thank you for the pull request, @jjspace!

✅ We can confirm we have a CLA on file for you.

Comment on lines +3 to +7
/* eslint-disable no-unused-vars */

const viewer = new Cesium.Viewer("cesiumContainer");

const blueBox = viewer.entities.add({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were a number of sandcastles that follow this structure of defining multiple examples and assigning them to variables that never actually get used. I thought about disabling no-unused-vars for all sandcastles but I do think it's a valid error we want to avoid. Instead I disabled it for these files specifically. I do think it's handy to have examples like these so it's easy for people to play with them if they want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant