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

fix: only use jquery once it is available #30571

Merged
merged 1 commit into from
Jan 10, 2022
Merged

fix: only use jquery once it is available #30571

merged 1 commit into from
Jan 10, 2022

Conversation

azul
Copy link
Contributor

@azul azul commented Jan 10, 2022

publicpage.js is loaded very early and cannot rely on jquery being loaded already.

Move the use of $ into the DomContentLoaded handler.

@azul
fix: only use jquery once it is available
94554c4 
publicpage.js is loaded very early and cannot rely on jquery being loaded already.

Move the use of `$` into the `DomContentLoaded` handler.

Signed-off-by: Azul <azul@riseup.net>
@azul azul force-pushed the fix/missing-jquery branch from 9a67578 to 94554c4 Compare January 10, 2022 13:55
@azul
Copy link
Contributor Author

azul commented Jan 10, 2022

Noticed this because cypress tests in text started failing complaining:

  > $ is not defined
https://on.cypress.io/uncaught-exception-from-application
      at http://localhost:8081/core/js/public/publicpage.js?v=16d6118e-0:36:1

This seems to be the easiest fix for now. Could also rewrite this without relying on jquery - but i guess that's for another time.

@azul azul requested review from skjnldsv and artonge January 10, 2022 13:57
@artonge
Copy link
Contributor

artonge commented Jan 10, 2022

/rebase

@skjnldsv skjnldsv merged commit 5faba45 into master Jan 10, 2022
@skjnldsv skjnldsv deleted the fix/missing-jquery branch January 10, 2022 19:03
@juliusknorr juliusknorr mentioned this pull request Jan 10, 2022
Merged
@solracsf
Copy link
Member

Backport?

@artonge
Copy link
Contributor

artonge commented Jan 11, 2022

/backport to stable23

@artonge
Copy link
Contributor

artonge commented Jan 11, 2022

/backport to stable22

@artonge
Copy link
Contributor

artonge commented Jan 11, 2022

/backport to stable21

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Jan 11, 2022
Merged
@backportbot-nextcloud
Copy link

The backport to stable21 failed. Please do this backport manually.

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Jan 11, 2022
Merged
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Jan 11, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artonge artonge artonge approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

Assignees
No one assigned
Labels
3. to review Waiting for reviews
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@azul @artonge @solracsf @skjnldsv