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(UnifiedSearch): request data only when opening modal #50241

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Jan 17, 2025

  • Resolves: #

Summary

Two requests are always being made on page opening/reload, even though search modal wasn't yet opened once (and there's no guarantee a user would use it until next page reload). So data should be fetched only when modal is actually opened

Checklist

@Antreesy Antreesy added this to the Nextcloud 31 milestone Jan 17, 2025
@Antreesy Antreesy requested review from susnux, nfebe and skjnldsv January 17, 2025 16:32
@Antreesy Antreesy self-assigned this Jan 17, 2025
@Antreesy Antreesy force-pushed the perf/noid/unified-search-init branch from e8171b3 to ebb170f Compare January 17, 2025 17:35
@Antreesy Antreesy force-pushed the perf/noid/unified-search-init branch from 5bca14e to 627b342 Compare January 17, 2025 17:42
Copy link
Member

@AndyScherzinger AndyScherzinger left a comment

Choose a reason for hiding this comment

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

🐘

@Antreesy
Copy link
Contributor Author

/compile /

Signed-off-by: nextcloud-command <[email protected]>
Comment on lines +320 to +322
.catch(error => {
console.error(error)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.catch(error => {
console.error(error)
})
.catch((error) => {
unifiedSearchLogger.error(error)
})

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

nit pick otherwise fine

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.

5 participants