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.

Sign up for GitHub

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

Jump to bottom

Properly save old route when opening the viewer #2322

Merged
merged 2 commits into from
Jun 4, 2024
Merged

Properly save old route when opening the viewer #2322

merged 2 commits into from
Jun 4, 2024

Conversation

artonge
Copy link
Contributor

@artonge artonge commented May 30, 2024 •
edited
Loading

I guess the params and query properties of the route are updated and not replaced on route change, so we were losing the previous state.

@artonge artonge force-pushed the artonge/fix/properly_save_current_state branch 2 times, most recently from cd3bd0d to 16d5e24 Compare May 30, 2024 10:28
@artonge artonge mentioned this pull request May 30, 2024
Closed
@artonge artonge requested review from susnux and skjnldsv May 30, 2024 10:28
@artonge artonge added bug Something isn't working 3. to review Waiting for reviews javascript Javascript related ticket labels May 30, 2024
@artonge artonge added this to the Nextcloud 30 milestone May 30, 2024
@artonge
Copy link
Contributor Author

artonge commented May 30, 2024

/backport to stable29

@artonge
Copy link
Contributor Author

artonge commented May 30, 2024

/backport to stable28

@backportbot backportbot bot added the backport-request Pending backport by the backport-bot label May 30, 2024
skjnldsv
skjnldsv approved these changes May 30, 2024
View reviewed changes
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

🐘

@artonge
Copy link
Contributor Author

artonge commented May 30, 2024

@skjnldsv, I don't have admin rights to bypass the unrelated failing Cypress test

@susnux
Copy link
Contributor

susnux commented May 30, 2024

I don't have admin rights to bypass the unrelated failing Cypress test

They are not really unrelated. Sometime ago a PR was merged without updating the snapshots.
You need to update the snapshots, then it should be good again.

PS: Yes probably a nextcloud-vue issue but tests are doing what they should do: Fail if something is wrong :)

@artonge
fix: Properly save old route when opening the viewer
09d754a 
Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge artonge force-pushed the artonge/fix/properly_save_current_state branch 2 times, most recently from b7096f8 to 5ab1372 Compare June 3, 2024 15:28
@artonge
Copy link
Contributor Author

artonge commented Jun 4, 2024

Updating snapshots did not help here. Not sure why they differ in the CI, they have some bold text.
In any case, this is not related at all to the change.

@susnux
Copy link
Contributor

susnux commented Jun 4, 2024

Updating snapshots did not help here. Not sure why they differ in the CI, they have some bold text. In any case, this is not related at all to the change.

Have generated them locally? I do not think this will work because of font issues. Best way is to download the snapshots from the CI workflow and use them.

@artonge @susnux
test(cypress): Update snapshots
80a0854 
Signed-off-by: Louis Chemineau <louis@chmn.me>
@susnux susnux force-pushed the artonge/fix/properly_save_current_state branch from 5ab1372 to 80a0854 Compare June 4, 2024 11:29
@susnux susnux merged commit 34d4d79 into master Jun 4, 2024
27 checks passed
@susnux susnux deleted the artonge/fix/properly_save_current_state branch June 4, 2024 11:42
@backportbot backportbot

This comment was marked as outdated.

Sign in to view
@backportbot backportbot

This comment was marked as outdated.

Sign in to view
@susnux
Copy link
Contributor

susnux commented Jun 4, 2024

/backport 09d754a to stable28

@susnux
Copy link
Contributor

susnux commented Jun 4, 2024

/backport 09d754a to stable29

@backportbot backportbot

This comment was marked as outdated.

Sign in to view
@backportbot backportbot bot mentioned this pull request Jun 4, 2024
Merged
2 tasks
@backportbot backportbot bot removed the backport-request Pending backport by the backport-bot label Jun 4, 2024
@artonge
Copy link
Contributor Author

artonge commented Jun 6, 2024

/backport 09d754a to stable28

@backportbot backportbot bot added the backport-request Pending backport by the backport-bot label Jun 6, 2024
@backportbot backportbot

This comment was marked as outdated.

Sign in to view
@artonge artonge mentioned this pull request Jun 6, 2024
Closed
@artonge artonge mentioned this pull request Jul 15, 2024
Merged
@blizzz blizzz mentioned this pull request Jul 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@susnux susnux susnux approved these changes

Assignees
No one assigned
Labels
3. to review Waiting for reviews backport-request Pending backport by the backport-bot bug Something isn't working javascript Javascript related ticket
Projects
None yet
Milestone
Nextcloud 30
Development

Successfully merging this pull request may close these issues.

[Bug]: Fail to create a second file in the Files web interface, unless I reload
3 participants
@artonge @susnux @skjnldsv