Skip to content

Plain javascript api to asynchronously load javascript or css files #13407

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
Merged
rullzer merged 3 commits into from
Jan 17, 2019
Merged

Plain javascript api to asynchronously load javascript or css files #13407

rullzer merged 3 commits into from
Jan 17, 2019

Conversation

@juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Jan 7, 2019 •
edited
Loading

This PR adds a javascript only public API to replace OC.addScript and OC.addStyle (which require jquery) to dynamically load js and css files after the initial loading.

Waiting for #13156 to integrate it into the webpack build.

Uses:

  • firstrunwizard: Load only when needed Move to vue firstrunwizard#106
  • richdocuments: Replace use of OC.addScript
  • files_texteditor: Replace use of OC.addScript

@juliusknorr juliusknorr added the 2. developing Work in progress label Jan 7, 2019
skjnldsv
skjnldsv reviewed Jan 7, 2019
View reviewed changes
This was referenced Jan 7, 2019
Closed
Merged
@rullzer
Copy link
Member

rullzer commented Jan 10, 2019

Waiting for #13156 to integrate it into the webpack build.

go go go 🚀

kesselb
kesselb reviewed Jan 10, 2019
View reviewed changes
@juliusknorr juliusknorr force-pushed the js-async-loading branch 2 times, most recently from 3298dba to 25e658c Compare January 15, 2019 17:11
@juliusknorr
Copy link
Member Author

juliusknorr commented Jan 15, 2019 •
edited
Loading

@ChristophWurst @skjnldsv I have moved the loader to the webpack build in core/ and made it extend the existing window.OCP object. Should we have this as a separate entrypoint and chunk or keep it in the one bundle for now?

Besides that, it is ready for review.

@juliusknorr juliusknorr added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 15, 2019
ChristophWurst
ChristophWurst reviewed Jan 15, 2019
View reviewed changes
@ChristophWurst
Copy link
Member

ChristophWurst commented Jan 15, 2019

@ChristophWurst @skjnldsv I have moved the loader to the webpack build in core/ and made it extend the existing window.OCP object. Should we have this as a separate entrypoint and chunk or keep it in the one bundle for now?

I think that is fine for now. We should be able to refactor that and move code around if necessary :)

@juliusknorr juliusknorr force-pushed the js-async-loading branch 3 times, most recently from 0dbe21f to b40dd20 Compare January 15, 2019 20:15
@rullzer
Copy link
Member

rullzer commented Jan 15, 2019

@ChristophWurst @skjnldsv I have moved the loader to the webpack build in core/ and made it extend the existing window.OCP object. Should we have this as a separate entrypoint and chunk or keep it in the one bundle for now?

I think that is fine for now. We should be able to refactor that and move code around if necessary :)

I agree. Basically the OCP stuff should always be loaded if possible anyway. So makes sense to have this in the dist pack.

kesselb
kesselb reviewed Jan 15, 2019
View reviewed changes
@juliusknorr juliusknorr added this to the Nextcloud 16 milestone Jan 15, 2019
@rullzer
Copy link
Member

rullzer commented Jan 16, 2019

So this can go in?

@juliusknorr
Copy link
Member Author

juliusknorr commented Jan 16, 2019

Yes, please review.

rullzer
rullzer approved these changes Jan 16, 2019
View reviewed changes
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Looks good to me and will make lazy loading much better :D

ChristophWurst
ChristophWurst approved these changes Jan 17, 2019
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Code looks good!

@ChristophWurst
Copy link
Member

ChristophWurst commented Jan 17, 2019

This will give a nice conflict with #13635 🤐

skjnldsv
skjnldsv reviewed Jan 17, 2019
View reviewed changes
core/js/js.js
if (msie > 0 || trident > 0) {
cssVars();
cssVars({
watch: true
Copy link
Member

@skjnldsv skjnldsv Jan 17, 2019

Choose a reason for hiding this comment

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

poor browser will die of exhaustion ⚰️

Copy link
Member

@rullzer rullzer Jan 17, 2019

Choose a reason for hiding this comment

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

pff so be it ;) No need to keep everybody stuck with crappy old stuff just ebcause of IE ;)

@MorrisJobke
Copy link
Member

MorrisJobke commented Jan 17, 2019

Please rebase and rebuild to resolve the conflict.

rullzer
rullzer previously requested changes Jan 17, 2019
View reviewed changes
@juliusknorr
Add public API for loading js files asynchronously
a36604d 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
Watch for changes with css variables polyfill
a86ee00 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
Bump bundled main.js
64b5c0c 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@rullzer rullzer merged commit ecc37f5 into master Jan 17, 2019
@ChristophWurst ChristophWurst mentioned this pull request Jan 17, 2019
Closed
@MorrisJobke MorrisJobke deleted the js-async-loading branch January 17, 2019 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@go2sh go2sh go2sh left review comments

@kesselb kesselb kesselb left review comments

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@rullzer rullzer rullzer left review comments

Assignees

No one assigned

Labels

3. to review Waiting for reviews technical debt

Projects

None yet

Milestone

Nextcloud 16

Development

Successfully merging this pull request may close these issues.

8 participants

@juliusknorr @rullzer @ChristophWurst @MorrisJobke @go2sh @kesselb @skjnldsv