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: use filename to find current index #2112

Merged
merged 2 commits into from Jan 9, 2024

Conversation

kesselb
Copy link
Contributor

@kesselb kesselb commented Dec 30, 2023

Fix #484

The viewer component is usually used with a folder and the basename unique.
However in photos, the fileList contains objects from multiple folders and the basename is not always unique.

How to reproduce:

  • Create folder A and B
  • Upload an image to folder A and name it "Test"
  • Upload another image to folder B and also name it "Test"
  • Open photos
  • Click on the image from folder B
  • The image from folder A is shown

To find the correct "currentIndex" we need filename (including the path).

My current understand is that fileInfo.filename should be always there, even if the remote file is not a dav file (e.g. attachments from mail). If that's not the case, we need to come up with an alternative solution.

@kesselb kesselb added enhancement New feature or request 3. to review Waiting for reviews labels Dec 30, 2023
@kesselb kesselb force-pushed the bug/484/find-current-index-by-filename branch 2 times, most recently from 0787a5c to 88a8731 Compare December 30, 2023 20:46
@kesselb
Copy link
Contributor Author

kesselb commented Dec 30, 2023

/compile amend /

@nextcloud-command nextcloud-command force-pushed the bug/484/find-current-index-by-filename branch from 88a8731 to 403853d Compare December 30, 2023 20:49
@kesselb kesselb added this to the Nextcloud 29 milestone Dec 30, 2023
@kesselb
Copy link
Contributor Author

kesselb commented Jan 3, 2024

/compile amend /

@nextcloud-command nextcloud-command force-pushed the bug/484/find-current-index-by-filename branch from 403853d to a6168ef Compare January 3, 2024 14:34
@kesselb kesselb force-pushed the bug/484/find-current-index-by-filename branch 2 times, most recently from 3fce515 to 761e86f Compare January 3, 2024 14:50
@kesselb
Copy link
Contributor Author

kesselb commented Jan 3, 2024

/compile /

@kesselb
Copy link
Contributor Author

kesselb commented Jan 3, 2024

/compile amend /

@nextcloud-command nextcloud-command force-pushed the bug/484/find-current-index-by-filename branch from 761e86f to ceb2c2f Compare January 3, 2024 16:34
@kesselb
Copy link
Contributor Author

kesselb commented Jan 3, 2024

I don't know what's wrong with the compile bot and/or CI.

The cypress tests are working locally.
I'm unable to compile the js files as expected.

@skjnldsv
Copy link
Member

skjnldsv commented Jan 5, 2024

Rebase and drop the compile changes

The viewer component is usually used with a folder and the basename unique.
However in photos, the fileList contains objects from multiple folders and the basename is not always unique.

How to reproduce:

- Create folder A and B
- Upload an image to folder A and name it "Test"
- Upload another image to folder B and also name it "Test"
- Open photos
- Click on the image from folder B
- The image from folder A is shown

To find the correct "currentIndex" we need filename (including the path).

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb kesselb force-pushed the bug/484/find-current-index-by-filename branch from ceb2c2f to 274818d Compare January 5, 2024 16:22
@kesselb
Copy link
Contributor Author

kesselb commented Jan 5, 2024

Rebase and drop the compile changes

Done ✔️

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 9, 2024
@skjnldsv
Copy link
Member

skjnldsv commented Jan 9, 2024

/compile

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@skjnldsv
Copy link
Member

skjnldsv commented Jan 9, 2024

let's see

@skjnldsv skjnldsv merged commit e31bc3e into master Jan 9, 2024
32 checks passed
@skjnldsv skjnldsv deleted the bug/484/find-current-index-by-filename branch January 9, 2024 18:08
@skjnldsv
Copy link
Member

skjnldsv commented Jan 9, 2024

/backport 274818d to stable28

@backportbot-nextcloud backportbot-nextcloud bot added the backport-request Pending backport by the backport-bot label Jan 9, 2024
@backportbot-nextcloud backportbot-nextcloud bot removed the backport-request Pending backport by the backport-bot label Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When clicking on photos with the same name the first photo will pop up
3 participants