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 events once #38385

Merged
merged 3 commits into from
Feb 10, 2021
Merged

Trigger events once #38385

merged 3 commits into from
Feb 10, 2021

Conversation

jvillafanez
Copy link
Member

@jvillafanez jvillafanez commented Feb 9, 2021

Description

Prevent the registration of the same event listener several times, which causes the same action (from the duplicated listeners) to be performed multiple times.

Primary problem is with the "urlChanged" event, which causes an ajax request to be thrown. This means that, without this fix, the ajax requests performed were increasing along with the increasing event listeners being registered for that event.
The other events don't seem to be too critical since the attached action is pretty light.

Related Issue

Helps with https://github.com/owncloud/enterprise/issues/4374

Motivation and Context

No need to register the same event listener twice

How Has This Been Tested?

Mostly tested going back and forth between the "all files" and "shared with you" sections in the files view, as well as checking there is no apparent problem with the scrolling in those sections.
Event registration has been checked with the firefox dev tools, ensuring there is no increasing event associated with the target web element. There could be multiple listeners for the same event, but it's expected to be for different purposes (not duplicated)

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@update-docs
Copy link

update-docs bot commented Feb 9, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

Copy link
Contributor

@JammingBen JammingBen left a comment

Choose a reason for hiding this comment

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

Didn't know that namespacing jQuery-events is a thing, cool stuff 👍

@karakayasemi
Copy link
Contributor

Looks good. Fixes #37085

@AlexAndBear AlexAndBear self-requested a review February 9, 2021 14:03
@sonarcloud
Copy link

sonarcloud bot commented Feb 9, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@enbrnz
Copy link
Contributor

enbrnz commented Mar 10, 2021

This patch was provided to a customer because of potential query optimization in https://github.com/owncloud/enterprise/issues/4374

The customer now reports that they discovered that they had to revert the patch, because the page "Shared with you" would no longer show all shares.

@micbar @jnweiger

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants