Skip to content

Fix delete not working in Dock#3805

Merged
ildyria merged 1 commit intomasterfrom
fix-delete-not-working
Nov 15, 2025
Merged

Fix delete not working in Dock#3805
ildyria merged 1 commit intomasterfrom
fix-delete-not-working

Conversation

@ildyria
Copy link
Copy Markdown
Member

@ildyria ildyria commented Nov 15, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Fixed text wrapping and layout alignment issues in the photo editor.
    • Improved handling of photo deletion operations with proper state synchronization.
    • Improved handling of photo move operations with enhanced data consistency.
    • Fixed album refresh behavior to properly update after deleting photos.

@ildyria ildyria requested a review from a team as a code owner November 15, 2025 09:31
@ildyria ildyria changed the title Fix delete not working Fix delete not working in Dock Nov 15, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 15, 2025

📝 Walkthrough

Walkthrough

This PR refactors photo gallery dialogs to integrate a centralized PhotoStore, replacing prop-dependent logic with store-aware decision paths for deletion and move operations. Additionally, it enhances the Album view's post-deletion refresh workflow and corrects a CSS responsive class and grid column syntax issue.

Changes

Cohort / File(s) Summary
UI & Layout Fixes
resources/js/components/drawers/PhotoEdit.vue
Updates responsive word-wrapping class from break-words to wrap-break-word and corrects grid column definition from minmax(auto, _1fr) to minmax(auto, 1fr) by removing invalid underscore.
Store Integration in Dialogs
resources/js/components/forms/gallery-dialogs/DeleteDialog.vue, resources/js/components/forms/gallery-dialogs/MoveDialog.vue
Introduces PhotoStore consumption via usePhotoStore() hook. Both dialogs now prioritize store-loaded photo data (title, ID) over props when available, routing confirmation and execution logic through photoStore.isLoaded state for cohesive photo vs. album operation selection.
Album View Deletion Workflow
resources/js/views/gallery-panels/Album.vue
Adds isDelete parameter to refresh() function. On DeleteDialog @deleted event, calls refresh(true) to trigger immediate album route navigation and early return; otherwise standard data reload occurs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Store integration logic: Review the conditional flows in DeleteDialog and MoveDialog to ensure store state checks correctly prioritize loaded data over props-based fallbacks.
  • Control flow coordination: Verify that the new isDelete parameter in Album.vue's refresh() function correctly handles both deletion and standard reload scenarios without unintended redirects.
  • PhotoStore assumptions: Confirm that photoStore.isLoaded, photoStore.photo.id, and photoStore.photo.title are guaranteed to exist when checked.

Poem

🐰 A hop, a fix, a store aligned,
Three dialogs now share one mind,
Photos move and delete with grace,
Store and props both find their place!
Album refreshes, swift as can be,
The gallery works in harmony! 🎨

Pre-merge checks

✅ 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.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fad833e and 78e12c7.

📒 Files selected for processing (4)
  • resources/js/components/drawers/PhotoEdit.vue (1 hunks)
  • resources/js/components/forms/gallery-dialogs/DeleteDialog.vue (4 hunks)
  • resources/js/components/forms/gallery-dialogs/MoveDialog.vue (5 hunks)
  • resources/js/views/gallery-panels/Album.vue (2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3721
File: resources/js/views/gallery-panels/Album.vue:200-204
Timestamp: 2025-09-28T12:43:29.852Z
Learning: The photoStore.load() method in resources/js/stores/PhotoState.ts already handles undefined photoId cases internally with a short circuit exit, so conditional checks before calling photoStore.load() are unnecessary.
📚 Learning: 2025-08-16T10:57:51.480Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3504
File: resources/js/views/gallery-panels/Albums.vue:11-13
Timestamp: 2025-08-16T10:57:51.480Z
Learning: In resources/js/views/gallery-panels/Albums.vue, the SelectDrag component is intentionally configured with :with-scroll="false" even though the #galleryView container has overflow-y-auto. This is working as intended according to the maintainer, who suggested that overflow-y-auto might be removed instead.

Applied to files:

  • resources/js/views/gallery-panels/Album.vue
📚 Learning: 2025-09-28T12:43:29.852Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3721
File: resources/js/views/gallery-panels/Album.vue:200-204
Timestamp: 2025-09-28T12:43:29.852Z
Learning: The photoStore.load() method in resources/js/stores/PhotoState.ts already handles undefined photoId cases internally with a short circuit exit, so conditional checks before calling photoStore.load() are unnecessary.

Applied to files:

  • resources/js/views/gallery-panels/Album.vue
  • resources/js/components/forms/gallery-dialogs/MoveDialog.vue
  • resources/js/components/forms/gallery-dialogs/DeleteDialog.vue
📚 Learning: 2025-09-28T08:27:38.332Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3721
File: resources/js/composables/album/photoActions.ts:39-43
Timestamp: 2025-09-28T08:27:38.332Z
Learning: The user ildyria (repository maintainer) prefers to skip error handling for the rotate photo functions (rotatePhotoCW and rotatePhotoCCW) in resources/js/composables/album/photoActions.ts.

Applied to files:

  • resources/js/views/gallery-panels/Album.vue
  • resources/js/components/forms/gallery-dialogs/MoveDialog.vue
  • resources/js/components/forms/gallery-dialogs/DeleteDialog.vue
🔇 Additional comments (12)
resources/js/views/gallery-panels/Album.vue (2)

58-58: LGTM! Correct handling for dock deletion.

The event binding correctly passes true to indicate a photo deletion from the dock view, which triggers the navigation logic in the refresh function. This is appropriately scoped to the photoStore-aware DeleteDialog (line 55 v-if="photoStore.isLoaded").


214-223: LGTM! Proper post-deletion navigation.

The refresh function correctly handles photo deletion from the dock by navigating back to the album view instead of attempting to reload the deleted photo. The early return prevents unnecessary store operations on the removed photo.

resources/js/components/forms/gallery-dialogs/MoveDialog.vue (4)

44-44: LGTM! Centralized photo store integration.

The photo store is properly imported and initialized, enabling the dialog to access the currently loaded photo from the dock/overlay view.

Also applies to: 56-56


100-104: LGTM! Proper prioritization of data sources.

The confirmation logic correctly prioritizes the photo store when loaded, falling back to props when not. This ensures the dialog uses the most current source of truth for the photo being moved.


106-114: Store access pattern is consistent with DeleteDialog.

The function correctly uses store data when available. The non-null assertions (photoStore!.photo!.title) assume that isLoaded guarantees photo is defined, which aligns with the store's design pattern seen throughout the codebase.


123-129: LGTM! Execution logic aligns with confirmation flow.

The move execution correctly follows the same prioritization as the confirmation logic, ensuring that store-based photo moves (from the dock) and prop-based moves (from bulk selection) both work correctly.

Also applies to: 163-174

resources/js/components/forms/gallery-dialogs/DeleteDialog.vue (4)

34-34: LGTM! Consistent store integration pattern.

The photo store integration matches the approach in MoveDialog, providing a unified source of truth for photo data across gallery dialogs.

Also applies to: 46-46


53-59: LGTM! Consistent confirmation logic across dialogs.

The confirmation computation follows the same pattern as MoveDialog, ensuring consistent behavior between move and delete operations.


61-69: LGTM! Store data correctly prioritized.

The delete confirmation function properly uses store data when the photo is loaded in the dock, maintaining consistency with the move dialog implementation.


78-86: LGTM! Execution flow properly handles both store and prop-based deletions.

The delete execution correctly distinguishes between store-based (dock) and prop-based (bulk selection) deletions, mirroring the move dialog's approach. This ensures the fix for dock deletions doesn't break existing bulk deletion functionality.

Also applies to: 106-114

resources/js/components/drawers/PhotoEdit.vue (2)

5-5: Good catch fixing the grid syntax!

The removal of the underscore in _1fr corrects an invalid CSS grid value. The grid column definition now properly uses minmax(auto,1fr) with valid CSS syntax.


3-3: Disregard the review comment—wrap-break-word is a valid Tailwind CSS v4 class.

The project uses Tailwind v4.1.0, and wrap-break-word is the standard utility class in Tailwind v4+ for the overflow-wrap: break-word property. The original review comment incorrectly flagged this as a non-existent custom class. The change from break-words (Tailwind v3 naming) to wrap-break-word (Tailwind v4 naming) is valid and appropriate for the project's current Tailwind version.

Likely an incorrect or invalid review comment.


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 and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.37%. Comparing base (fad833e) to head (78e12c7).

🚀 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.

@ildyria
Copy link
Copy Markdown
Member Author

ildyria commented Nov 15, 2025

@coderabbitai review please?

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 15, 2025

@ildyria: I'll review the changes in this PR that fixes the delete functionality in Dock.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ildyria ildyria enabled auto-merge (squash) November 15, 2025 11:01
@ildyria ildyria merged commit 310a2a5 into master Nov 15, 2025
45 checks passed
@ildyria ildyria deleted the fix-delete-not-working branch November 15, 2025 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants