Conversation
📝 WalkthroughWalkthroughMultiple docs updated; new gallery filter translations added across languages; photo rating UI and store filtering introduced; selection system migrated from index-based to ID-based across components, composables, and stores; several Vue components adjusted to emit/accept IDs instead of numeric indices. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
7833a26 to
6c8ed71
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
6c8ed71 to
6a3caf0
Compare
643dad4 to
c543baa
Compare
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (8)
lang/de/gallery.php (1)
64-67: Consider localizing the new filter strings for German UI consistency.
If German translations are available, replacing the English labels will keep the locale experience consistent with other translated entries.lang/es/gallery.php (1)
65-68: Consider localizing the new filter strings for Spanish UI consistency.
If translations are available, replacing the English labels will improve the Spanish locale experience.lang/vi/gallery.php (1)
65-68: Consider localizing the new filter strings for Vietnamese UI consistency.
If translations are available, replacing the English labels will improve the Vietnamese locale experience.resources/js/components/gallery/albumModule/PhotoThumbPanelControl.vue (1)
3-7: Keep the filter controls visible when a filter is active.
IfphotoRatingFilterstays set whilehasRatedPhotosis false, the UI disappears and users can’t clear the filter. Consider gating on either condition.♻️ Suggested adjustment
- v-if="photosStore.hasRatedPhotos" + v-if="photosStore.hasRatedPhotos || photosStore.photoRatingFilter !== null"resources/js/composables/album/dragAndSelect.ts (1)
230-242: Consider indexing selectables to avoid O(n²) lookups.For large albums, repeated
findcalls can get expensive. A small Map-based index keeps this O(n).♻️ Proposed refactor
function reduceIntersection<Model extends { id: string }>( intersection: string[], selectables: Model[], validator: (i: Model) => boolean, ): string[] { - return intersection.reduce((result: string[], id) => { - const item = selectables.find((p) => p.id === id && validator(p)); - if (item) { - result.push(item.id); - } - return result; - }, []); + const byId = new Map(selectables.map((item) => [item.id, item])); + return intersection.reduce((result: string[], id) => { + const item = byId.get(id); + if (item && validator(item)) { + result.push(item.id); + } + return result; + }, []); }resources/js/views/gallery-panels/Favourites.vue (1)
92-104: Consider converting async/await to .then() pattern.As per coding guidelines, Vue3 components should avoid
async/awaitand use.then()instead. This applies to theonMountedcallback.♻️ Suggested refactor
-onMounted(async () => { - const results = await Promise.allSettled([layoutStore.load()]); - - results.forEach((result, index) => { - if (result.status === "rejected") { - console.warn(`Promise ${index} reject with reason: ${result.reason}`); - } - }); - - if (results.every((result) => result.status === "fulfilled")) { - setScroll(); - } -}); +onMounted(() => { + Promise.allSettled([layoutStore.load()]).then((results) => { + results.forEach((result, index) => { + if (result.status === "rejected") { + console.warn(`Promise ${index} reject with reason: ${result.reason}`); + } + }); + + if (results.every((result) => result.status === "fulfilled")) { + setScroll(); + } + }); +});resources/js/stores/ModalsState.ts (1)
57-58: Consider renaming for consistency.The properties
nonHoverSelectablePhotosIdxandnonHoverSelectableAlbumsIdxstore string IDs (per the type annotationstring[]), but theIdxsuffix suggests numeric indices. For consistency with the newselectedPhotosIds/selectedAlbumsIdsnaming, consider renaming these tononHoverSelectablePhotosIdsandnonHoverSelectableAlbumsIds.resources/js/components/gallery/albumModule/AlbumThumbPanelList.vue (1)
33-45: Prefer function declarations for handlers.Switch
maySelectandmenuOpento function declarations per Vue coding guidelines.♻️ Suggested refactor
-const maySelect = (id: string, e: MouseEvent) => { +function maySelect(id: string, e: MouseEvent) { if (props.isInteractive === false) { return; } emits("clicked", id, e); -}; +} -const menuOpen = (id: string, e: MouseEvent) => { +function menuOpen(id: string, e: MouseEvent) { if (props.isInteractive === false) { return; } emits("contexted", id, e); -}; +}As per coding guidelines, avoid
const fn = () => {}in Vue components.
Summary by CodeRabbit
New Features
Documentation
Localization
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.