Fix Masonry (and other layout not being respected)#4184
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded a loading guard and spinner to the album panel UI; refactored album loading to sync layout, run photo/album loads in parallel via a loader array and Promise.all, and introduced pagination state and getters for photos and albums. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/js/components/gallery/albumModule/AlbumPanel.vue (1)
17-34:⚠️ Potential issue | 🔴 CriticalCritical:
v-if="true"makes the spinner permanently visible andnoDatablock unreachable.The condition
v-if="true"is a constant that always evaluates to true, causing:
- The spinner to always render, regardless of actual loading state
- The
v-else-if="noData"block (lines 22-34) to be completely unreachable dead code- Users will never see the "no results" message or the upload button
This appears to be debug/placeholder code. The condition should likely check the actual loading state.
🐛 Proposed fix to show spinner only while loading
- <div v-if="true"> + <div v-if="albumStore.isLoading || albumStore.photos_loading || albumStore.albums_loading"> <div class="flex w-full h-full items-center justify-center"> <i class="pi pi-spin pi-spinner text-4xl text-primary-400" /> </div> </div>
🧹 Nitpick comments (1)
resources/js/stores/AlbumState.ts (1)
349-363: Missing semicolon on line 352.The parallel loading logic is well-structured, firing
loadPhotosimmediately and conditionally addingloadAlbumsfor model albums. However, line 352 is missing a trailing semicolon.🔧 Proposed fix
this.config = data.data.config; layoutStore.layout = data.data.config.photo_layout; - const loader = [this.loadPhotos(1, false)] + const loader = [this.loadPhotos(1, false)]; if (data.data.config.is_model_album) {
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 34c62a80-2f9a-4c39-a359-3aadb2bd4f61
📒 Files selected for processing (2)
resources/js/components/gallery/albumModule/AlbumPanel.vueresources/js/stores/AlbumState.ts
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc80da93-65ea-453c-bf04-d4e8ee3e82c5
📒 Files selected for processing (2)
resources/js/components/gallery/albumModule/AlbumPanel.vueresources/js/stores/AlbumState.ts
Fixes #4183
Summary by CodeRabbit
New Features
Improvements
Bug Fixes