Conversation
… support local theme paths
…prevent overwriting existing themes
…detection in createTheme function
…dd peerDependencies for common package
…heme instructions for clarity
…hance layout with responsive image handling
…nction to include components directory
…mline package resolutions
… with scaffolding instructions
…and documentation
… paths for common types
…ace package details
…ility functions for gallery integration
… of the base theme structure and customization options
…omprehensive structure, components, and configuration files
…ructions in theme-related documentation
…o feature/themes-support
… new import paths
… loadGalleryData function
…ckage API details
… gallery customization options
…lidation improvements
…ority and user overrides
haltakov
left a comment
There was a problem hiding this comment.
So, while playing with these I have a suggestion how to make all commands and setting be more consistent...
The general rule should be that all theme settings (theme path, thumb size and thumb edge mode) should follow this hierarchy (which is partially implemented already):
CLI options > gallery.json config > default theme config > global defaults
Note that this is not the case for the theme param yet.
How this works across the different CLI commands:
init- we shouldn't ask things the user probably has no idea yet. Even more, we currently ask about the thumb size but no the theme. I suggest to leave it as before without asking, but allow the user to set--theme,--thumbnail-sizeand--thumbnail-edgethat then stored in thegallery.jsonconfig. This is only for users that already know what they are doing with the themethumbnails- to be honest, I'm even thinking of removing this command as we can have justbuild. Not 100% sure yet. I suggest here we also allow the user to set--thumbnail-sizeand--thumbnail-edgeif they want tobuild- again, we allow the user to set--theme,--thumbnail-sizeand--thumbnail-edgewhich then take precedence over thegallery.json
gallery/src/modules/create-theme/templates/base/src/layouts/MainLayout.astro
Outdated
Show resolved
Hide resolved
...es/modern/src/features/themes/base-theme/components/gallery-section/GallerySectionItem.astro
Outdated
Show resolved
Hide resolved
...es/modern/src/features/themes/base-theme/components/gallery-section/GallerySectionItem.astro
Show resolved
Hide resolved
themes/modern/src/features/themes/base-theme/components/lightbox/PhotoSwipe.astro
Outdated
Show resolved
Hide resolved
gallery/src/modules/create-theme/templates/base/src/components/GalleryItem.astro
Outdated
Show resolved
Hide resolved
…il settings, clarifying CLI options and hierarchy
… for gallery customization
… lightbox functionality with deep linking support
…umbnail configuration with CLI options for size and edge application
…ts by utilizing pre-computed hero srcsets; remove unused props for cleaner code
|
Changes introduced:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3a8311eea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const galleryJsonPath = process.env.GALLERY_JSON_PATH || './gallery.json'; | ||
| const galleryData = JSON.parse(fs.readFileSync(galleryJsonPath, 'utf8')); | ||
| // Load and resolve gallery data - all paths are pre-computed | ||
| const galleryJsonPath = import.meta.env.GALLERY_JSON_PATH || './gallery.json'; |
There was a problem hiding this comment.
Read gallery.json path from env in Astro build
This now reads import.meta.env.GALLERY_JSON_PATH, but the build only defines process.env.GALLERY_JSON_PATH in astro.config.ts. In Vite/Astro, import.meta.env only contains prefixed or explicitly defined vars, so this will be undefined during spg build, causing the code to fall back to ./gallery.json in the theme directory and loadGalleryData to throw when that file isn’t there. Use process.env.GALLERY_JSON_PATH here or add a Vite define for import.meta.env.GALLERY_JSON_PATH to avoid breaking CLI builds.
Useful? React with 👍 / 👎.
Resolves: #62
Summary
Documentation updates and theme scaffolder fixes to improve the developer experience for contributors and theme creators.
Changes
Documentation
create-themedocs/themes.mdwith clearer instructions and development workflowTheme Scaffolder (
spg create-theme)@simple-photo-gallery/commoninstead of@simple-photo-gallery/common/src/...)Import Changes
@simple-photo-gallery/common/src/gallery) to package entrypoint imports (@simple-photo-gallery/common)Important: Build
commonFirstThe
commonpackage must be built before TypeScript/ESLint can resolve@simple-photo-gallery/commonimports. This is because:dist/(notsrc/)After cloning the repo, run:
This ensures proper type resolution and avoids "Cannot find module" errors.
Creating and Building Themes Locally
To create and test a theme during development:
From the gallery directory
cd galleryCreate a new theme
yarn gallery create-theme my-themeBuild a gallery with your theme (using local CLI)
yarn gallery build --theme ../themes/my-theme -g /path/to/galleryThis uses the local development CLI (yarn gallery) instead of the published spg command.