Skip to content
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

fix(server): library control doesn't apply to new library from the third row #4331

Merged
merged 1 commit into from Oct 3, 2023

Conversation

alextran1502
Copy link
Contributor

@alextran1502 alextran1502 commented Oct 3, 2023

Fixes #4327

Fixed the issue of the index value in the #for-each loop doesn't update when passing in the function call, leading to library's content from the third row cannot be modified

The solution is to use the local variable of the component that keeps track of the state of the selected library and the library's index, then use that information in the function call.

Cause: Unknown - Svelte's magic

@vercel
Copy link

vercel bot commented Oct 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
immich ⬜️ Ignored (Inspect) Visit Preview Oct 3, 2023 5:59pm

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

I think we have had several issues like this, where we have a context menu inside of a for loop and it generally leads to some problems. Why can't we just have the Context Menu be defined at the bottom of the file? It isn't using anything scoped anymore. Also, why do we need to keep track of both the selected index and selected library? Can we only keep track of the selected library instead?

@alextran1502
Copy link
Contributor Author

@jrasm91 This is the current drop-in replacement for the initial implementation of this editing menu. We have a plan to move this to a separate modal, which has all the modification options on a single view for better UX. So the fix will ensure everything is still working until I roll out the modal design

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

Sounds good.

@alextran1502 alextran1502 merged commit 02f9b40 into main Oct 3, 2023
21 checks passed
@alextran1502 alextran1502 deleted the fix/4327 branch October 3, 2023 19:05
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.

[BUG] External Libraries: Not able to edit the upper one if more than one gallery available
2 participants