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

Create a login bundle for login page scripts #13929

Merged
merged 1 commit into from May 7, 2019
Merged

Create a login bundle for login page scripts #13929

merged 1 commit into from May 7, 2019

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Jan 30, 2019 •
edited

Once done this will make it obsolete to load 95% or the scripts we currently load on the login page, meaning that users will see faster page loads when the open the login page on a (new) device with empty caches.

@ChristophWurst ChristophWurst added this to the Nextcloud 16 milestone Jan 30, 2019
@ChristophWurst ChristophWurst self-assigned this Jan 30, 2019
@ChristophWurst ChristophWurst added this to IN PROGRESS (max 3 PRs) in Christoph's Tasks via automation Jan 30, 2019
@MorrisJobke
Copy link
Member

Once done this will make it obsolete to load 95% or the scripts we currently load on the login page, meaning that users will see faster page loads when the open the login page on a (new) device with empty caches.

Or after an update 😉

@ChristophWurst ChristophWurst moved this from IN PROGRESS (max 3 PRs) to BLOCKED in Christoph's Tasks Jan 31, 2019
@rullzer
Copy link
Member

rullzer commented Jan 31, 2019

🚀

Of course a good second step would be to add something that does preloading of required files for after the login. Once the login page is done loading.

But first this yes!

@rullzer rullzer modified the milestones: Nextcloud 16, Nextcloud 17 Feb 25, 2019
@ChristophWurst ChristophWurst added 1. to develop Accepted and waiting to be taken care of and removed 2. developing Work in progress labels Mar 6, 2019
@ChristophWurst ChristophWurst moved this from BLOCKED to SELECTED in Christoph's Tasks Mar 8, 2019
@ChristophWurst ChristophWurst added 2. developing Work in progress and removed 1. to develop Accepted and waiting to be taken care of labels Apr 30, 2019
@ChristophWurst ChristophWurst moved this from SELECTED to IN PROGRESS (max 3 PRs) in Christoph's Tasks Apr 30, 2019
@ChristophWurst ChristophWurst moved this from IN PROGRESS (max 3 PRs) to BLOCKED in Christoph's Tasks Apr 30, 2019
@ChristophWurst
Copy link
Member Author

ChristophWurst commented May 2, 2019 •
edited

Almost there 😁

  • BUG: clicking the lost password element triggers an error, OC.getRootPath is still added via js.js
    Bildschirmfoto von 2019-05-02 15-49-13

@ChristophWurst
Copy link
Member Author

Tested and works 🚀 💃 🎊

@ChristophWurst ChristophWurst added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 7, 2019
@ChristophWurst ChristophWurst moved this from BLOCKED to TO REVIEW (max 4 PRs) in Christoph's Tasks May 7, 2019
skjnldsv
skjnldsv approved these changes May 7, 2019
View reviewed changes
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Wow!! Yeeeeesss!! 🚀 🚀 🙏

Christoph's Tasks automation moved this from TO REVIEW (max 4 PRs) to TO INTEGRATE May 7, 2019
MorrisJobke
MorrisJobke approved these changes May 7, 2019
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Works, but for me the main.js with 1,2 MB is still loaded. Is this correct?

@MorrisJobke
Copy link
Member

Also before it was 32 requests with 1,7 MB in total and after it is 29 requests with 2,1 MB. 🤔

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels May 7, 2019
@ChristophWurst
Copy link
Member Author

Works, but for me the main.js with 1,2 MB is still loaded. Is this correct?

I actually haven't checked recently. But it could very well be the case as it's just loaded everywhere.

@ChristophWurst
Copy link
Member Author

It's loaded for every template

server/lib/private/legacy/template.php

Line 121 in 65964a5

OC_Util::addScript('core', 'dist/main', true);
. This has to be adjusted slightly to only load a predefined set of scripts for the different types of templates we have.

@ChristophWurst @MorrisJobke
Create a login bundle for login page scripts
3cc8c9b 
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@MorrisJobke
Copy link
Member

I fixed the conflicts here.

@MorrisJobke
Copy link
Member

Looks good except the git clone problems. Merging now.

@MorrisJobke MorrisJobke merged commit ee6011d into master May 7, 2019
Christoph's Tasks automation moved this from TO INTEGRATE to DONE May 7, 2019
@MorrisJobke MorrisJobke deleted the refactor/login-bundle branch May 7, 2019 13:11
@icewind1991
Copy link
Member

This change breaks password reset

@icewind1991 icewind1991 mentioned this pull request Jul 24, 2019
Closed
@rullzer rullzer mentioned this pull request Jul 25, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@rullzer rullzer Awaiting requested review from rullzer

@georgehrke georgehrke Awaiting requested review from georgehrke

Assignees

@ChristophWurst ChristophWurst

Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement technical debt
Projects
No open projects
Christoph's Tasks
DONE
Milestone
Nextcloud 17
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ChristophWurst @MorrisJobke @rullzer @icewind1991 @skjnldsv