Skip to content

Improve loading screens - #4576

Merged
ildyria merged 26 commits into
masterfrom
loading
Aug 5, 2026
Merged

Improve loading screens#4576
ildyria merged 26 commits into
masterfrom
loading

Conversation

@ildyria

@ildyria ildyria commented Jul 28, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Added an optional design-system showcase with configurable theme colors.
    • Added guided face-cluster review with skip, undo, shortcuts, and dismissal confirmations.
    • Added photo removal from person profiles with localized confirmation dialogs.
  • Improvements

    • Introduced refreshed loading indicators, page transitions, and clearer error feedback.
    • Improved setup submission handling and prevented duplicate submissions.
    • Updated gallery, footer, favicon, buttons, and interface styling.
    • Expanded translations for theme settings and face-management actions.

@ildyria ildyria added the v8 label Jul 28, 2026
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR adds the Lychee v8 design system, configurable semantic theme colors, a design-system showcase, shared loading components, improved loading-state handling, face-review workflows, and related interface updates.

Changes

Lychee v8 design system

Layer / File(s) Summary
Theme configuration and localization
DESIGN.md, app/Http/..., database/migrations/..., lang/*
Adds design-token documentation, theme-color settings, palette generation, capability data, migration support, and localized strings.
Design-system showcase and theme rendering
resources/js/v8/views/admin/DesignSystem.vue, resources/js/v8/router/..., resources/sass/app-v8.css, resources/views/components/...
Adds the design-system route and showcase, configurable palette output, and semantic color mappings.
Frontend verification checklist
ui.todo, .gitignore
Adds manual v8 interface checks and ignore rules for local design-tool artifacts.

Loading and interface behavior

Layer / File(s) Summary
Shared loading component rollout
resources/js/v8/components/LycheeLoadingIcon.vue, resources/js/v8/components/**, resources/js/v8/views/**
Adds LycheeLoadingIcon and replaces Spinner across loading states.
Full-screen loading and readiness
resources/js/v8/components/loading/LoadingProgress.vue, resources/js/v8/views/**
Adds loading overlays and asynchronous error handling for view initialization.
Forms, transitions, and layout
resources/js/v8/views/AdminSetupPage.vue, resources/js/v8/views/App.vue, resources/js/v8/components/**
Adds guarded submission, route transitions, semantic button styles, gallery layout updates, and footer integration.

Face workflows

Layer / File(s) Summary
Face review and photo removal
resources/js/v8/views/face-recog/*, resources/js/v8/components/forms/people/*, lang/*/people.php
Replaces cluster details with a review queue and adds confirmation, undo, keyboard navigation, pagination updates, localized workflow text, and confirmed photo removal.

Estimated code review effort: 5 (Critical) | ~120 minutes

Poem

A rabbit checks each glowing hue,
And swaps old spinners for a view.
Face clusters line up in a queue,
Removal asks before it’s due.
New tokens bloom across the night—
“Hop!” says the rabbit, “v8 is bright.”

🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ildyria
ildyria marked this pull request as ready for review August 1, 2026 20:02
@ildyria
ildyria requested a review from a team as a code owner August 1, 2026 20:02
Comment thread .github/skills/impeccable/scripts/live/svelte-component.mjs Fixed
Comment thread .github/skills/impeccable/scripts/live/svelte-component.mjs Fixed
Comment thread .github/skills/impeccable/scripts/live/svelte-component.mjs Fixed
Comment thread .github/skills/impeccable/scripts/live/svelte-component.mjs Fixed
Comment thread .github/skills/impeccable/scripts/live/svelte-component.mjs Fixed
Comment thread .github/skills/impeccable/scripts/live-accept.mjs Fixed
Comment thread .github/skills/impeccable/scripts/detector/shared/page.mjs Fixed
Comment thread .github/skills/impeccable/scripts/context.mjs Fixed
Comment thread .github/skills/impeccable/scripts/lib/is-generated.mjs Fixed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 8

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
DESIGN.md (1)

186-189: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add the required "Last updated" footer.

This file is documentation under **/*.md. Add an hr line followed by "Last updated: [date]" at the bottom of the file. This file currently ends without it.

As per coding guidelines: "Use Markdown format for documentation. At the bottom of documentation files, add an hr line followed by "Last updated: [date of the update]"".

📝 Proposed fix
 - **Don't** let the interface read as a generic self-hosted admin panel — boxy unstyled tables, default form chrome, or dashboard sprawl for its own sake is the thing this system is explicitly designed against.
+
+---
+*Last updated: 2026-07-28*

Source: Coding guidelines

🟡 Minor comments (14)
ui.todo-52-52 (1)

52-52: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the typo "LigtBox".

Line 52 has a typo: "LigtBox (lightbox)". Correct it to "LightBox" if that is the actual component name, or align it with the actual component name used in the codebase.

✏️ Proposed fix
-- [ ] Flow view (/flow, /flow/:albumId/:photoId): AlbumCard, CarouselImages, LigtBox (lightbox),
+- [ ] Flow view (/flow, /flow/:albumId/:photoId): AlbumCard, CarouselImages, LightBox (lightbox),
ui.todo-1-2 (1)

1-2: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add NUXT_UI_ENABLED as a real, testable environment/config entry.

config/features.php defines the nuxt_ui feature behind env('NUXT_UI_ENABLED', false), but there is no usable NUXT_UI_ENABLED entry in the repo for non-spec files. Add it to .env.example or .env so testers can follow the checklist instead of hunting behind the spec docs.

Source: Coding guidelines

resources/js/v8/views/App.vue-49-64 (1)

49-64: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Honor the reduced-motion preference.

The route transition still runs for users who set prefers-reduced-motion: reduce. Disable the opacity and transform transition in that media query.

Proposed fix
+.@media (prefers-reduced-motion: reduce) {
+	.lychee-page-enter-active,
+	.lychee-page-leave-active {
+		transition: none;
+	}
+}
DESIGN.md-27-32 (1)

27-32: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the inconsistent lineHeight unit for the label typography token.

The title and body tokens use unitless line-height multipliers (1.4, 1.5). The label token uses 0.8rem, a length value. Use the same unit convention for lineHeight across all typography tokens.

DESIGN.md-4-15 (1)

4-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add the missing flag color to the front-matter colors block.

The prose at line 98 describes 5 flag roles: nsfw, trophy, person, rated, and link (public-hidden, #fb923c). The front-matter colors block only lists flag-nsfw, flag-trophy, flag-person, and flag-rated. Add the missing link/public-hidden entry so the structured token list matches the documented flag palette.

lang/en/all_settings.php-398-405 (1)

398-405: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Rename the visible accent terminology.

These entries configure primary_color, but their labels and help text still describe an accent color. Use primary-color terminology consistently.

  • lang/en/all_settings.php#L398-L405: Rename Accent color. to Primary color.
  • lang/en/all_settings.php#L795-L802: Replace stale accent color references with primary color.
  • lang/en/all_settings.php#L864-L864: Refer to the primary color above.
  • lang/de/all_settings.php#L396-L403: Rename Accent color. to Primary color.
  • lang/de/all_settings.php#L793-L800: Replace stale accent color references with primary color.
  • lang/de/all_settings.php#L860-L860: Refer to the primary color above.
  • lang/el/all_settings.php#L398-L405: Rename Accent color. to Primary color.
  • lang/el/all_settings.php#L795-L802: Replace stale accent color references with primary color.
  • lang/el/all_settings.php#L864-L864: Refer to the primary color above.
  • lang/es/all_settings.php#L396-L403: Rename Accent color. to Primary color.
  • lang/es/all_settings.php#L793-L800: Replace stale accent color references with primary color.
  • lang/es/all_settings.php#L860-L860: Refer to the primary color above.

The provided backend context maps the primary token to primary_color.

resources/js/v8/components/LycheeLoadingIcon.vue-55-130 (1)

55-130: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Honor reduced-motion preferences.

The loading icon animates indefinitely when the user requests reduced motion. Add a reduced-motion rule that stops the animation and keeps every stage visible.

Proposed fix
+@media (prefers-reduced-motion: reduce) {
+	.lychee-loading-stage {
+		animation: none;
+		opacity: 1;
+	}
+}
lang/sk/all_settings.php-833-833 (1)

833-833: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use a snake_case config category key for Theme Colors.

lang/en/all_settings.php and each locale duplicate the Theme Colors key directly from database/migrations/2026_08_01_000000_add_theme_colors.php::CAT. Rename the source CAT to theme_colors, then update the migration and all lang/<locale>/all_settings.php entries so translation source keys follow the PHP coding-convention requirement.

Source: Coding guidelines

lang/zh_TW/all_settings.php-398-398 (1)

398-398: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use Primary color consistently.

The key is primary_color, but Line [398] still displays Accent color.. Line [795] still describes the fallback as the default accent color. This makes the renamed setting ambiguous for administrators. Update both strings to use primary color, or keep the key name accent_color in every producer and consumer.

Proposed wording fix
-        'primary_color' => 'Accent color.',
+        'primary_color' => 'Primary color',

-        'primary_color' => 'Pick a colour, the nearest colour palette will be used. If you leave this empty, the default accent color will be used.<br>Note: you will need to refresh the page to see the changes.',
+        'primary_color' => 'Pick a colour, the nearest colour palette will be used. If you leave this empty, the default primary color will be used.<br>Note: you will need to refresh the page to see the changes.',

Also applies to: 795-795

resources/js/v8/components/loading/LoadingProgress.vue-5-5 (1)

5-5: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use standard Tailwind classes for the overlay background and blur.

LoadingProgress.vue uses light:bg-white/60 and backdrop:backdrop-blur-2xl, but resources/sass/app-v8.css does not define light or backdrop custom variants. The v7 imports keep their own variants, so this v8 overlay component will miss the light-mode background and blur. Use bg-white/60 dark:bg-black/60 backdrop-blur-2xl instead.

resources/js/v8/views/gallery-panels/Flow.vue-22-22 (1)

22-22: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

flex justify-center is applied to the loading icon instead of a container. LycheeLoadingIcon uses an <svg> as its root element, so class bindings land on that <svg>. justify-content affects only that element's own children, so the icon is not centered. Wrap the icon in a centering container at both sites.

  • resources/js/v8/views/gallery-panels/Flow.vue#L22-L22: move flex justify-center to a wrapper div that carries the v-if, and give the icon an explicit size such as text-2xl.
  • resources/js/v8/views/gallery-panels/Timeline.vue#L55-L55: move flex justify-center to a wrapper div that carries the v-if, and keep text-2xl on the icon.
resources/js/v8/views/webshop/BasketList.vue-2-2 (1)

2-2: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Handle failed loading in loading sentinels.

LoadingProgress stays active when order or options remains undefined. lycheeStateStore.load() clears through .catch(), but the new shop loaders do not: useStepOne.loadCheckoutOptions() only assigns data on success, and orderStore.load() only assigns this.order on success. Make these load failures reach a terminal false state in the loaded flags, so rejected requests do not leave the overlay showing forever.

resources/js/v8/components/forms/settings/ColorField.vue-5-5 (1)

5-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Replace <UChip> with a documented design-system exception.

docs/specs/3-reference/coding-conventions.md requires <template> first, then <script lang="ts">; move <script> before <style>. The getDefaultColor(key) helper also uses props.config.key for the current selection.

Source: Coding guidelines

resources/js/v8/components/forms/settings/ColorField.vue-63-68 (1)

63-68: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Give the neutral_color case its own lexical scope.

const body is declared directly in the switch body here, and Biome’s noSwitchDeclarations rule treats switch cases as a shared scope. Wrap this case in braces so the body query variable cannot leak or collide with other cases.

Proposed fix
-		case "neutral_color":
-			const b = document.querySelector("body");
-			if (b?.classList.contains("dark")) {
+		case "neutral_color": {
+			const body = document.querySelector("body");
+			if (body?.classList.contains("dark")) {
 				return "`#71717b`"; // dark mode: zinc color
 			}
 			return "`#62748e`"; // or slate color
+		}

Source: Linters/SAST tools

🧹 Nitpick comments (4)
app/View/Components/Style.php (1)

41-56: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Consider batching the palette cache lookups.

This constructor now performs up to 7 separate Cache::rememberForever calls per request instead of 1. Since this component renders on most page loads, this adds cache round-trips per request. Collect the non-empty colors first, then fetch them with Cache::many() and generate and store only the missing palettes in one batch, instead of looking up each token individually.

resources/js/v8/views/admin/DesignSystem.vue (1)

281-302: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Resolved hex values do not refresh after a theme change.

registerSwatch reads the computed background color one time, when Vue binds the element ref. If a palette custom property changes while this view stays mounted, the swatch color updates through CSS, but the printed hex text stays stale. Consider recomputing on demand, for example inside onMounted plus a watcher on the palette source.

resources/js/v8/views/face-recog/FaceClusters.vue (1)

383-396: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Replace async/await with promise chaining.

fetchAllClusterFaceIds uses async and await. The project guidelines require .then() in Vue 3 code. Convert the paging loop to a recursive promise chain.

As per coding guidelines: "Do not use await async calls in Vue3, use .then() instead".

Source: Coding guidelines

resources/js/v8/components/forms/settings/ColorField.vue (1)

78-83: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Use the supplied configuration for the default lookup.

stringToBlossomColorPickerValue accepts config, but the fallback still reads props.config.key. Use config.key so the helper remains correct when it receives a different configuration object.

Proposed fix
-		color = getDefaultColor(props.config.key);
+		color = getDefaultColor(config.key);

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 110ed523-55e1-4882-8756-9200240f1343

📥 Commits

Reviewing files that changed from the base of the PR and between 92ac7e1 and c0d78f9.

⛔ Files ignored due to path filters (2)
  • public/img/logo4.svg is excluded by !**/*.svg
  • resources/img/logo4.svg is excluded by !**/*.svg
📒 Files selected for processing (135)
  • DESIGN.md
  • app/Http/Controllers/Admin/SettingsController.php
  • app/Http/Resources/Rights/ModulesRightsResource.php
  • app/View/Components/Style.php
  • database/migrations/2026_07_23_000000_add_accent_color.php
  • database/migrations/2026_08_01_000000_add_theme_colors.php
  • lang/ar/all_settings.php
  • lang/ar/people.php
  • lang/bg/all_settings.php
  • lang/bg/people.php
  • lang/cz/all_settings.php
  • lang/cz/people.php
  • lang/de/all_settings.php
  • lang/de/people.php
  • lang/el/all_settings.php
  • lang/el/people.php
  • lang/en/all_settings.php
  • lang/en/design-system.php
  • lang/en/people.php
  • lang/es/all_settings.php
  • lang/es/people.php
  • lang/fa/all_settings.php
  • lang/fa/people.php
  • lang/fr/all_settings.php
  • lang/fr/people.php
  • lang/hu/all_settings.php
  • lang/hu/people.php
  • lang/it/all_settings.php
  • lang/it/people.php
  • lang/ja/all_settings.php
  • lang/ja/people.php
  • lang/nl/all_settings.php
  • lang/nl/people.php
  • lang/no/all_settings.php
  • lang/no/people.php
  • lang/pl/all_settings.php
  • lang/pl/people.php
  • lang/pt/all_settings.php
  • lang/pt/people.php
  • lang/ru/all_settings.php
  • lang/ru/people.php
  • lang/sk/all_settings.php
  • lang/sk/people.php
  • lang/sv/all_settings.php
  • lang/sv/people.php
  • lang/tr/all_settings.php
  • lang/tr/people.php
  • lang/vi/all_settings.php
  • lang/vi/people.php
  • lang/zh_CN/all_settings.php
  • lang/zh_CN/people.php
  • lang/zh_TW/all_settings.php
  • lang/zh_TW/people.php
  • resources/js/lychee.d.ts
  • resources/js/router/paths.ts
  • resources/js/v8/components/LycheeLoadingIcon.vue
  • resources/js/v8/components/Spinner.vue
  • resources/js/v8/components/drawers/PhotoDetails.vue
  • resources/js/v8/components/footers/GalleryFooter.vue
  • resources/js/v8/components/forms/album/AlbumShare.vue
  • resources/js/v8/components/forms/album/AlbumVisibility.vue
  • resources/js/v8/components/forms/album/ApplyRenamerDialog.vue
  • resources/js/v8/components/forms/basic/InputPassword.vue
  • resources/js/v8/components/forms/people/RemovePhotoFromPersonDialog.vue
  • resources/js/v8/components/forms/settings/ColorField.vue
  • resources/js/v8/components/gallery/PersonCard.vue
  • resources/js/v8/components/gallery/albumModule/AlbumPanel.vue
  • resources/js/v8/components/gallery/photoModule/PhotoRatingWidget.vue
  • resources/js/v8/components/loading/LoadingProgress.vue
  • resources/js/v8/components/maintenance/DuplicateLine.vue
  • resources/js/v8/components/maintenance/MaintenanceBackfillAlbumSizes.vue
  • resources/js/v8/components/maintenance/MaintenanceBulkScanFaces.vue
  • resources/js/v8/components/maintenance/MaintenanceBulkScanNsfw.vue
  • resources/js/v8/components/maintenance/MaintenanceCleaning.vue
  • resources/js/v8/components/maintenance/MaintenanceDestroyDismissedFaces.vue
  • resources/js/v8/components/maintenance/MaintenanceDuplicateChecker.vue
  • resources/js/v8/components/maintenance/MaintenanceFilesize.vue
  • resources/js/v8/components/maintenance/MaintenanceFixJobs.vue
  • resources/js/v8/components/maintenance/MaintenanceFixTree.vue
  • resources/js/v8/components/maintenance/MaintenanceFlushCache.vue
  • resources/js/v8/components/maintenance/MaintenanceFlushQueue.vue
  • resources/js/v8/components/maintenance/MaintenanceFulfillOrders.vue
  • resources/js/v8/components/maintenance/MaintenanceFulfillPrecompute.vue
  • resources/js/v8/components/maintenance/MaintenanceGenSizevariants.vue
  • resources/js/v8/components/maintenance/MaintenanceMissingPalettes.vue
  • resources/js/v8/components/maintenance/MaintenanceOldOrders.vue
  • resources/js/v8/components/maintenance/MaintenanceOptimize.vue
  • resources/js/v8/components/maintenance/MaintenanceResetFaceScanStatus.vue
  • resources/js/v8/components/maintenance/MaintenanceRunClustering.vue
  • resources/js/v8/components/maintenance/MaintenanceSyncFaceEmbeddings.vue
  • resources/js/v8/components/maintenance/StatisticsIntegrity.vue
  • resources/js/v8/components/modals/CameraCapture.vue
  • resources/js/v8/components/modals/ImportFromServer.vue
  • resources/js/v8/components/modals/faceRecog/SelfieClaimModal.vue
  • resources/js/v8/components/pagination/PaginationInfiniteScroll.vue
  • resources/js/v8/components/settings/General.vue
  • resources/js/v8/composables/useAdminTiles.ts
  • resources/js/v8/router/routes.ts
  • resources/js/v8/views/AdminSetupPage.vue
  • resources/js/v8/views/App.vue
  • resources/js/v8/views/BulkAlbumEdit.vue
  • resources/js/v8/views/ChangeLogs.vue
  • resources/js/v8/views/Landing.vue
  • resources/js/v8/views/LoginPage.vue
  • resources/js/v8/views/Permissions.vue
  • resources/js/v8/views/RegisterPage.vue
  • resources/js/v8/views/RenamerRules.vue
  • resources/js/v8/views/Sharing.vue
  • resources/js/v8/views/TagsManagement.vue
  • resources/js/v8/views/admin/AdminDashboard.vue
  • resources/js/v8/views/admin/ContactMessages.vue
  • resources/js/v8/views/admin/DesignSystem.vue
  • resources/js/v8/views/admin/Moderation.vue
  • resources/js/v8/views/admin/NsfwConfig.vue
  • resources/js/v8/views/admin/Settings.vue
  • resources/js/v8/views/admin/UserGroups.vue
  • resources/js/v8/views/admin/Users.vue
  • resources/js/v8/views/admin/Webhooks.vue
  • resources/js/v8/views/face-recog/FaceClusters.vue
  • resources/js/v8/views/face-recog/FaceMaintenance.vue
  • resources/js/v8/views/face-recog/People.vue
  • resources/js/v8/views/face-recog/PersonDetail.vue
  • resources/js/v8/views/gallery-panels/Favourites.vue
  • resources/js/v8/views/gallery-panels/Flow.vue
  • resources/js/v8/views/gallery-panels/Map.vue
  • resources/js/v8/views/gallery-panels/Timeline.vue
  • resources/js/v8/views/webshop/BasketList.vue
  • resources/js/v8/views/webshop/CheckoutPage.vue
  • resources/js/v8/views/webshop/OrderDownload.vue
  • resources/sass/app-v8.css
  • resources/views/components/meta.blade.php
  • resources/views/components/style.blade.php
  • resources/views/vueapp.blade.php
  • routes/web_v2.php
  • ui.todo
💤 Files with no reviewable changes (1)
  • resources/js/v8/components/Spinner.vue

Comment thread database/migrations/2026_08_01_000000_add_theme_colors.php
Comment thread resources/js/v8/components/forms/basic/InputPassword.vue
Comment thread resources/js/v8/views/admin/Settings.vue
Comment thread resources/js/v8/views/face-recog/FaceClusters.vue
Comment thread resources/js/v8/views/gallery-panels/Map.vue Outdated
Comment thread resources/js/v8/views/Landing.vue
Comment thread resources/js/v8/views/LoginPage.vue
@codecov

codecov Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.02%. Comparing base (30aeba5) to head (9c87e18).

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
resources/js/v8/views/face-recog/FaceClusters.vue (1)

600-606: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

openQueue triggers loadQueueClusterFaces twice.

openQueue sets queueIndex.value = idx, then calls loadQueueClusterFaces() directly. Setting queueIndex.value also changes it from null to idx. The watch(queueIndex, ...) at Line 719 fires because queueVisible.value is already true by the time the watcher runs (both assignments in openQueue complete synchronously before the pre-flush watcher executes). The watcher then calls loadQueueClusterFaces() again.

Each call resets detailFaces.value, detailCurrentPage.value, and issues a new getClusterFaces request. Every queue open fires two identical requests instead of one.

Remove the direct call in openQueue and let the watcher handle it, since skipQueueCluster already relies solely on the watcher for this purpose.

🐛 Proposed fix to remove the redundant call
 function openQueue(cluster: App.Http.Resources.Models.ClusterPreviewResource) {
 	const idx = clusters.value.findIndex((c) => c.cluster_label === cluster.cluster_label);
 	if (idx === -1) return;
 	queueIndex.value = idx;
 	queueVisible.value = true;
-	loadQueueClusterFaces();
 }

Also applies to: 719-723


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fd978c93-76e4-446b-85db-ee90bf1957bf

📥 Commits

Reviewing files that changed from the base of the PR and between c91837a and 6bb0c6b.

📒 Files selected for processing (19)
  • resources/js/v8/components/forms/album/AlbumShare.vue
  • resources/js/v8/components/forms/settings/ColorField.vue
  • resources/js/v8/components/loading/LoadingProgress.vue
  • resources/js/v8/components/renamer/RenamerRuleLine.vue
  • resources/js/v8/components/settings/ConfirmSave.vue
  • resources/js/v8/components/settings/CssJs.vue
  • resources/js/v8/components/settings/General.vue
  • resources/js/v8/views/Landing.vue
  • resources/js/v8/views/LoginPage.vue
  • resources/js/v8/views/Permissions.vue
  • resources/js/v8/views/RegisterPage.vue
  • resources/js/v8/views/Sharing.vue
  • resources/js/v8/views/admin/Settings.vue
  • resources/js/v8/views/admin/Users.vue
  • resources/js/v8/views/face-recog/FaceClusters.vue
  • resources/js/v8/views/gallery-panels/Flow.vue
  • resources/js/v8/views/gallery-panels/Map.vue
  • resources/js/v8/views/gallery-panels/Timeline.vue
  • resources/js/v8/views/webshop/BasketList.vue
🚧 Files skipped from review as they are similar to previous changes (11)
  • resources/js/v8/components/forms/album/AlbumShare.vue
  • resources/js/v8/components/loading/LoadingProgress.vue
  • resources/js/v8/views/gallery-panels/Timeline.vue
  • resources/js/v8/views/Permissions.vue
  • resources/js/v8/views/Sharing.vue
  • resources/js/v8/views/admin/Settings.vue
  • resources/js/v8/views/admin/Users.vue
  • resources/js/v8/components/settings/General.vue
  • resources/js/v8/components/forms/settings/ColorField.vue
  • resources/js/v8/views/gallery-panels/Flow.vue
  • resources/js/v8/views/gallery-panels/Map.vue

Comment thread resources/js/v8/components/settings/CssJs.vue
Comment thread resources/js/v8/views/Landing.vue

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
resources/js/v8/views/face-recog/FaceClusters.vue (3)

135-171: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Block repeated face toggles while a request is pending.

The face-removal control stays enabled while dismissSingleFace waits for FaceDetectionService.toggleDismissed. A double click can send two toggle requests. The second request can restore the face after the first request dismisses it, while both success handlers remove the face and decrement face_count.

Track pending face IDs. Disable the remove control and the undo action until the corresponding request completes.

The supplied dismissal flow and FaceDetectionService confirm that this action toggles dismissed state. (raw.githubusercontent.com)

Also applies to: 727-800

Source: MCP tools


606-622: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Guard queue face loads against stale responses.

openQueue starts a queue load immediately, while the queueIndex watcher starts another load after queueIndex.value = idx changes. skipQueueCluster starts a new cluster load before the previous load resolves. loadQueueClusterFaces() then applies each response to detailFaces, detailCurrentPage, and detailHasMorePages without checking that the response still matches the requested cluster_label, so slow responses can replace the current cluster’s face list with stale faces.

Correlate pending face loads with queueCluster.value.cluster_label before applying success updates and before enabling pagination/loading flags in finally.

Source: MCP tools


361-508: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Reconcile partial batch mutations.

performBatchDismiss uses Promise.all for per-cluster fetches and dismisses, while undoDismiss uses Promise.all for per-face restores. Any failure can leave dismissed/undismissed faces out of sync with the local queue, and confirmDismiss still closes the dialog. Use an atomic bulk operation, or handle each operation with Promise.allSettled, reconcile successful changes before closing, and add a test for one success with one failure.

Source: MCP tools


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 277e2ae3-0407-4864-a419-36e4080de820

📥 Commits

Reviewing files that changed from the base of the PR and between b198c9b and ada1d37.

📒 Files selected for processing (65)
  • resources/js/v8/components/forms/group/AddUserGroupModal.vue
  • resources/js/v8/components/forms/photo/PhotoTagDialog.vue
  • resources/js/v8/components/forms/tags/TagDeleteDialog.vue
  • resources/js/v8/components/gallery/albumModule/AlbumHeaderPanel.vue
  • resources/js/v8/components/maintenance/MaintenanceDuplicateChecker.vue
  • resources/js/v8/components/maintenance/MaintenanceFixJobs.vue
  • resources/js/v8/components/maintenance/MaintenanceFixTree.vue
  • resources/js/v8/components/maintenance/MaintenanceFlushCache.vue
  • resources/js/v8/components/maintenance/MaintenanceFlushQueue.vue
  • resources/js/v8/components/maintenance/MaintenanceFulfillOrders.vue
  • resources/js/v8/components/maintenance/MaintenanceFulfillPrecompute.vue
  • resources/js/v8/components/maintenance/MaintenanceGenSizevariants.vue
  • resources/js/v8/components/maintenance/MaintenanceMissingPalettes.vue
  • resources/js/v8/components/maintenance/MaintenanceOldOrders.vue
  • resources/js/v8/components/maintenance/MaintenanceOptimize.vue
  • resources/js/v8/components/maintenance/MaintenanceResetFaceScanStatus.vue
  • resources/js/v8/components/maintenance/MaintenanceRunClustering.vue
  • resources/js/v8/components/maintenance/MaintenanceSyncFaceEmbeddings.vue
  • resources/js/v8/components/maintenance/MaintenanceUpdate.vue
  • resources/js/v8/components/maintenance/StatisticsIntegrity.vue
  • resources/js/v8/components/modals/AboutLychee.vue
  • resources/js/v8/components/modals/CameraCapture.vue
  • resources/js/v8/components/modals/ConfirmModalHost.vue
  • resources/js/v8/components/modals/DownloadAlbum.vue
  • resources/js/v8/components/modals/DownloadPhoto.vue
  • resources/js/v8/components/modals/ImportFromLink.vue
  • resources/js/v8/components/modals/ImportFromServer.vue
  • resources/js/v8/components/modals/RegisterLychee.vue
  • resources/js/v8/components/modals/SecurityAdvisoriesModal.vue
  • resources/js/v8/components/modals/SessionExpiredReload.vue
  • resources/js/v8/components/modals/UploadPanel.vue
  • resources/js/v8/components/modals/WebauthnModal.vue
  • resources/js/v8/components/modals/faceRecog/BatchFaceAssignmentModal.vue
  • resources/js/v8/components/modals/faceRecog/FaceAssignmentModal.vue
  • resources/js/v8/components/modals/faceRecog/MergePersonModal.vue
  • resources/js/v8/components/modals/faceRecog/SelfieClaimModal.vue
  • resources/js/v8/components/pagination/PaginationLoadMore.vue
  • resources/js/v8/components/renamer/RenamerRuleModal.vue
  • resources/js/v8/components/settings/ConfirmSave.vue
  • resources/js/v8/components/settings/General.vue
  • resources/js/v8/components/webshop/Disclaimer.vue
  • resources/js/v8/components/webshop/InfoSection.vue
  • resources/js/v8/components/webshop/ThankYou.vue
  • resources/js/v8/views/AdminSetupPage.vue
  • resources/js/v8/views/BulkAlbumEdit.vue
  • resources/js/v8/views/Contact.vue
  • resources/js/v8/views/DuplicatesFinder.vue
  • resources/js/v8/views/FixTree.vue
  • resources/js/v8/views/RegisterPage.vue
  • resources/js/v8/views/RenamerRules.vue
  • resources/js/v8/views/Sharing.vue
  • resources/js/v8/views/admin/ContactMessages.vue
  • resources/js/v8/views/admin/Moderation.vue
  • resources/js/v8/views/admin/UserGroups.vue
  • resources/js/v8/views/admin/Users.vue
  • resources/js/v8/views/admin/WatermarkPreview.vue
  • resources/js/v8/views/admin/Webhooks.vue
  • resources/js/v8/views/admin/shop/PrintPixelSizesAdmin.vue
  • resources/js/v8/views/face-recog/FaceClusters.vue
  • resources/js/v8/views/face-recog/FaceMaintenance.vue
  • resources/js/v8/views/face-recog/People.vue
  • resources/js/v8/views/webshop/BasketList.vue
  • resources/js/v8/views/webshop/CheckoutPage.vue
  • resources/js/v8/views/webshop/OrderDownload.vue
  • resources/js/v8/views/webshop/OrderList.vue
🚧 Files skipped from review as they are similar to previous changes (34)
  • resources/js/v8/views/admin/Users.vue
  • resources/js/v8/components/maintenance/MaintenanceFixTree.vue
  • resources/js/v8/components/modals/ImportFromServer.vue
  • resources/js/v8/components/forms/group/AddUserGroupModal.vue
  • resources/js/v8/views/RegisterPage.vue
  • resources/js/v8/components/maintenance/StatisticsIntegrity.vue
  • resources/js/v8/components/maintenance/MaintenanceMissingPalettes.vue
  • resources/js/v8/components/modals/faceRecog/SelfieClaimModal.vue
  • resources/js/v8/views/RenamerRules.vue
  • resources/js/v8/views/admin/Webhooks.vue
  • resources/js/v8/components/maintenance/MaintenanceRunClustering.vue
  • resources/js/v8/views/admin/Moderation.vue
  • resources/js/v8/components/maintenance/MaintenanceFulfillOrders.vue
  • resources/js/v8/components/maintenance/MaintenanceGenSizevariants.vue
  • resources/js/v8/views/face-recog/People.vue
  • resources/js/v8/components/maintenance/MaintenanceOptimize.vue
  • resources/js/v8/views/admin/UserGroups.vue
  • resources/js/v8/components/maintenance/MaintenanceOldOrders.vue
  • resources/js/v8/components/maintenance/MaintenanceFlushQueue.vue
  • resources/js/v8/views/AdminSetupPage.vue
  • resources/js/v8/components/maintenance/MaintenanceResetFaceScanStatus.vue
  • resources/js/v8/components/maintenance/MaintenanceFulfillPrecompute.vue
  • resources/js/v8/components/maintenance/MaintenanceFlushCache.vue
  • resources/js/v8/components/modals/CameraCapture.vue
  • resources/js/v8/views/Sharing.vue
  • resources/js/v8/components/maintenance/MaintenanceDuplicateChecker.vue
  • resources/js/v8/views/webshop/OrderDownload.vue
  • resources/js/v8/components/settings/ConfirmSave.vue
  • resources/js/v8/views/BulkAlbumEdit.vue
  • resources/js/v8/views/webshop/BasketList.vue
  • resources/js/v8/components/settings/General.vue
  • resources/js/v8/components/maintenance/MaintenanceSyncFaceEmbeddings.vue
  • resources/js/v8/components/maintenance/MaintenanceFixJobs.vue
  • resources/js/v8/views/face-recog/FaceMaintenance.vue

@ildyria ildyria changed the title Loading Improve loading screens Aug 5, 2026
@ildyria
ildyria merged commit a6f45c0 into master Aug 5, 2026
49 checks passed
@ildyria
ildyria deleted the loading branch August 5, 2026 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants