-
-
Notifications
You must be signed in to change notification settings - Fork 362
fix: Use correct base URL for embedded galleries #3833
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
Conversation
📝 WalkthroughWalkthroughThe EmbedCodeDialog component now imports a Constants module and uses Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
resources/js/components/forms/album/EmbedCodeDialog.vue (2)
219-223:apiUrlcomputation correctly handles non‑root base paths (optional normalization centralization)Switching from
window.location.origintoConstants.BASE_URLand trimming trailing slashes with.replace(/\/+$/, "")is a solid fix for instances mounted under a sub‑path and avoids double slashes when appending/embed. As a minor nit, if other call sites also need a normalized base URL, consider moving the trailing‑slash normalization into the constants service itself so callers can just use a pre‑normalizedBASE_URL.
384-388: Simplified preview error message is fine; consider future i18n / dev‑hint tweakThe shorter
"Failed to load preview."message is clear and has no functional implications. If you expect preview failures to be common in dev (e.g., embed assets not built/served) or want better localization, you might later move this string into the i18n bundle and/or include a brief hint that deployment/asset build can affect preview availability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
resources/js/components/forms/album/EmbedCodeDialog.vue(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-28T08:39:34.280Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3721
File: resources/js/stores/NsfwConsentedState.ts:3-5
Timestamp: 2025-09-28T08:39:34.280Z
Learning: In Lychee codebase, all Pinia store files follow the pattern of declaring the type alias before the store definition (e.g., `export type StoreType = ReturnType<typeof useStore>; export const useStore = defineStore(...)`). This pattern compiles successfully with TypeScript 5.9 and vue-tsc.
Applied to files:
resources/js/components/forms/album/EmbedCodeDialog.vue
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: 2️⃣ PHP tests / 8.3 - sqlite -- Install
- GitHub Check: 2️⃣ PHP tests / 8.3 - sqlite -- ImageProcessing
- GitHub Check: 2️⃣ PHP tests / 8.3 - sqlite -- Feature_v2
- GitHub Check: 2️⃣ PHP tests / 8.3 - sqlite -- Webshop
- GitHub Check: 2️⃣ PHP tests / 8.3 - sqlite -- Unit
- GitHub Check: 2️⃣ PHP tests / 8.3 - postgresql -- ImageProcessing
- GitHub Check: 2️⃣ PHP tests / 8.3 - postgresql -- Webshop
- GitHub Check: 2️⃣ PHP tests / 8.3 - mariadb -- ImageProcessing
- GitHub Check: 2️⃣ PHP tests / 8.3 - postgresql -- Feature_v2
- GitHub Check: 2️⃣ PHP tests / 8.3 - mariadb -- Webshop
- GitHub Check: 2️⃣ PHP tests / 8.3 - postgresql -- Unit
- GitHub Check: 2️⃣ PHP tests / 8.3 - mariadb -- Unit
- GitHub Check: 2️⃣ PHP tests / 8.3 - mariadb -- Feature_v2
- GitHub Check: 2️⃣ PHP 8.3 - License Check
- GitHub Check: 2️⃣ PHP 8.3 - Code Style errors
- GitHub Check: 2️⃣ PHP 8.3 - PHPStan
🔇 Additional comments (1)
resources/js/components/forms/album/EmbedCodeDialog.vue (1)
143-143: Importing sharedConstantsservice is appropriate hereUsing
Constantskeeps URL configuration centralized and consistent with the rest of the app; no issues with this addition.
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
This PR should fix embedded galleries for Lychee instances hosted at non-root paths (e.g.
example.com/photosinstead ofphotos.example.com).Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.