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

Trigger "changeDirectory" event on URL change #30593

Merged
merged 1 commit into from
Jan 12, 2022

Conversation

PVince81
Copy link
Member

When using the browser back button or clicking on sections on the left
sidebar (like favorites), the "changeDirectory" jQuery event did not get
called, so apps like recommendations would not notice the directory
change.

This fixes the issue by also setting changeUrl to true when the file
list's directory got changed as a result from a URL change.

Note that in the past this argument was there to prevent repushing the
same URL to the history stack but is obsolete already as there's a check
in place to not push again whatever is already there.

Fixes #19048
I've tested this PR with both scenarios from the ticket.

@PVince81 PVince81 added bug 3. to review Waiting for reviews labels Jan 11, 2022
@PVince81 PVince81 self-assigned this Jan 11, 2022
@PVince81 PVince81 changed the title Trigger "changeDirectory" even on URL change Trigger "changeDirectory" event on URL change Jan 11, 2022
@PVince81
Copy link
Member Author

oh, I need to adjust the JS tests

Copy link
Contributor

@miaulalala miaulalala left a comment

Choose a reason for hiding this comment

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

As er our debug sesh - LGTM :)

When using the browser back button or clicking on sections on the left
sidebar (like favorites), the "changeDirectory" jQuery event did not get
called, so apps like recommendations would not notice the directory
change.

This fixes the issue by also setting changeUrl to true when the file
list's directory got changed as a result from a URL change.

Added optional changedThroughUrl argument to make sure the event
recipient knows if the change was done through a URL change and make it
possible prevent a loop in the onDirectoryChange handler that actually
changes the URL when the origin was already from a URL change.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@PVince81 PVince81 force-pushed the bugfix/19048/fix-changedirectory-recommendations branch from 1ca7165 to 809e305 Compare January 11, 2022 16:48
@PVince81
Copy link
Member Author

I had to resort to a more complicated fix to prevent the "URL changed -> then change URL" loop which broke the tests and is potentially dangerous.

please re-review

@PVince81 PVince81 requested a review from miaulalala January 11, 2022 16:49
@PVince81
Copy link
Member Author

/backport to stable23

@PVince81
Copy link
Member Author

/backport to stable22

@PVince81
Copy link
Member Author

/backport to stable21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accessing folder via favorites quick links in navigation keeps wrong Workspace and Recommendations
4 participants