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

Improving performance of Web app #1105

Closed
aayusharora opened this Issue Mar 1, 2017 · 25 comments

Comments

4 participants
@aayusharora
Member

aayusharora commented Mar 1, 2017

  • Please see Google page insights and improve the performance of pages.
  • Approaches we can adopt for RAIL model.
  • Suggest and implement the ways to improve DOM and CSSOM construction, which is currently done at once.
  • Remove render-blocking CSS
  • Add gulp to minify the JS, CSS code for production.
  • Compress images and scale them to 264px rather than 300px.
  • Add lazy loading for images
@Princu7

This comment has been minimized.

Show comment
Hide comment
@Princu7

Princu7 Mar 2, 2017

Contributor

I will read up on Gulp and try to integrate it. Will also compress the images using sharp module.

Contributor

Princu7 commented Mar 2, 2017

I will read up on Gulp and try to integrate it. Will also compress the images using sharp module.

@geekyd

This comment has been minimized.

Show comment
Hide comment
@geekyd

geekyd Mar 6, 2017

Member

@Princu7 If you have not started working on minification yet, can I work on the minifying css and js using gulp?

Member

geekyd commented Mar 6, 2017

@Princu7 If you have not started working on minification yet, can I work on the minifying css and js using gulp?

@Princu7

This comment has been minimized.

Show comment
Hide comment
@Princu7

Princu7 Mar 6, 2017

Contributor

@geekyd Yeah, sure. Go ahead 👍

Contributor

Princu7 commented Mar 6, 2017

@geekyd Yeah, sure. Go ahead 👍

@geekyd

This comment has been minimized.

Show comment
Hide comment
@geekyd

geekyd Mar 8, 2017

Member

@aayusharora I'm working on minification of the scripts and css files using gulp, will be sending a PR for it.

Member

geekyd commented Mar 8, 2017

@aayusharora I'm working on minification of the scripts and css files using gulp, will be sending a PR for it.

@geekyd

This comment has been minimized.

Show comment
Hide comment
@geekyd

geekyd Mar 8, 2017

Member

@aayusharora I have implemented the minification of all the css, js and html files using gulp.
Please check : https://open-event-web-appp.herokuapp.com
Also check the performance using https://developers.google.com/speed/pagespeed/insights/
The score has improved. Thanks

Member

geekyd commented Mar 8, 2017

@aayusharora I have implemented the minification of all the css, js and html files using gulp.
Please check : https://open-event-web-appp.herokuapp.com
Also check the performance using https://developers.google.com/speed/pagespeed/insights/
The score has improved. Thanks

@aayusharora

This comment has been minimized.

Show comment
Hide comment
@aayusharora

aayusharora Mar 8, 2017

Member

@geekyd Thanks. Minification is working but I guess, still there are 7 JS files which are requested as resources and same goes for 3 CSS files. When we give the app to production, we can send only 1 minified JS and CSS from Node server, Please make sure there will be no conflicts in the code when doing this. You can use IIFE in JS to remove same name variable/function conflicts.

Member

aayusharora commented Mar 8, 2017

@geekyd Thanks. Minification is working but I guess, still there are 7 JS files which are requested as resources and same goes for 3 CSS files. When we give the app to production, we can send only 1 minified JS and CSS from Node server, Please make sure there will be no conflicts in the code when doing this. You can use IIFE in JS to remove same name variable/function conflicts.

@geekyd

This comment has been minimized.

Show comment
Hide comment
@geekyd

geekyd Mar 8, 2017

Member

@aayusharora So i should concat all the Js files and create a master script for web-app and same with Css??

Member

geekyd commented Mar 8, 2017

@aayusharora So i should concat all the Js files and create a master script for web-app and same with Css??

@aayusharora

This comment has been minimized.

Show comment
Hide comment
@aayusharora

aayusharora Mar 8, 2017

Member

Hmm, let's leave CSS for now, planning to use critical-css module to only load necessary CSS on first load. JS we can minify and do.

Member

aayusharora commented Mar 8, 2017

Hmm, let's leave CSS for now, planning to use critical-css module to only load necessary CSS on first load. JS we can minify and do.

@geekyd

This comment has been minimized.

Show comment
Hide comment
@geekyd

geekyd Mar 8, 2017

Member

@aayusharora Okay I'm on it. Thanks :)

Member

geekyd commented Mar 8, 2017

@aayusharora Okay I'm on it. Thanks :)

@aayusharora

This comment has been minimized.

Show comment
Hide comment
@aayusharora

aayusharora Mar 12, 2017

Member

@geekyd Updates ?

Member

aayusharora commented Mar 12, 2017

@geekyd Updates ?

@geekyd

This comment has been minimized.

Show comment
Hide comment
@geekyd

geekyd Mar 12, 2017

Member

@aayusharora I'm almost done with this, fixing the js files as they are causing problems after using iife and uglify. I will send a PR soon.

Member

geekyd commented Mar 12, 2017

@aayusharora I'm almost done with this, fixing the js files as they are causing problems after using iife and uglify. I will send a PR soon.

@mariobehling

This comment has been minimized.

Show comment
Hide comment
@mariobehling

mariobehling Mar 13, 2017

Member

I am totally for speed up but lazy loading is distracting the user and makes this site look slower. It also means: Once I open the page, I do not download the whole page and can go offline, but it becomes more of an online app. The goal of this app is rather to have the browser storing the app and making the images smaller than lazy loading (where I need to be online).
We achieved to make the FOSSASIA 2016 Summit to 4.2 MB
For the FOSSASIA 2017 Summit just the background image is still 1.8MB.
So, can we rather optimize on that end than lazy loading?

Member

mariobehling commented Mar 13, 2017

I am totally for speed up but lazy loading is distracting the user and makes this site look slower. It also means: Once I open the page, I do not download the whole page and can go offline, but it becomes more of an online app. The goal of this app is rather to have the browser storing the app and making the images smaller than lazy loading (where I need to be online).
We achieved to make the FOSSASIA 2016 Summit to 4.2 MB
For the FOSSASIA 2017 Summit just the background image is still 1.8MB.
So, can we rather optimize on that end than lazy loading?

@geekyd

This comment has been minimized.

Show comment
Hide comment
@geekyd

geekyd Mar 13, 2017

Member

@mariobehling @aayusharora @Princu7 Lazy load can be customised to provide a better user experience, please check this link https://www.appelsiini.net/projects/lazyload , we can customise the Threshold , to load pictures before it reaches the viewport.

Member

geekyd commented Mar 13, 2017

@mariobehling @aayusharora @Princu7 Lazy load can be customised to provide a better user experience, please check this link https://www.appelsiini.net/projects/lazyload , we can customise the Threshold , to load pictures before it reaches the viewport.

@Princu7

This comment has been minimized.

Show comment
Hide comment
@Princu7

Princu7 Mar 13, 2017

Contributor

@geekyd Yes. Increasing the threshold would be better indeed. But one more problem with the lazy loading as stated by @mariobehling is that the pages won't load fully unless we completely explore the page which means that they can't be saved and then viewed offline properly at conferences and events. We should have more detailed discussion regarding this but for the time being, let's go with the current configuration. Thanks!!

Contributor

Princu7 commented Mar 13, 2017

@geekyd Yes. Increasing the threshold would be better indeed. But one more problem with the lazy loading as stated by @mariobehling is that the pages won't load fully unless we completely explore the page which means that they can't be saved and then viewed offline properly at conferences and events. We should have more detailed discussion regarding this but for the time being, let's go with the current configuration. Thanks!!

@geekyd

This comment has been minimized.

Show comment
Hide comment
@geekyd

geekyd Mar 14, 2017

Member

@aayusharora I propose not to concat all the JS files of the project as not all the JS files are used in every page. If we concat all the js it leads to error in script for the page where it is not used.
eg.
tabs.js gives error in the index.html page as it cannot resolve the classes used in the js.

We should minify all the js and use them separately.What is your opinion. Would love to know your opinion. :)

Member

geekyd commented Mar 14, 2017

@aayusharora I propose not to concat all the JS files of the project as not all the JS files are used in every page. If we concat all the js it leads to error in script for the page where it is not used.
eg.
tabs.js gives error in the index.html page as it cannot resolve the classes used in the js.

We should minify all the js and use them separately.What is your opinion. Would love to know your opinion. :)

@aayusharora

This comment has been minimized.

Show comment
Hide comment
@aayusharora

aayusharora Mar 14, 2017

Member

@mariobehling reverting this will load, say 1000 images at once on Network, lazy loading is always effective and efficient. Nobody wants to use a slow app.
@mariobehling @Princu7 @geekyd I would suggest to resume this and make changes such that it works before the viewport.
@mariobehling The concept of lazy loading does not depend on internet, hence there is no problem for this in offline mode.

Member

aayusharora commented Mar 14, 2017

@mariobehling reverting this will load, say 1000 images at once on Network, lazy loading is always effective and efficient. Nobody wants to use a slow app.
@mariobehling @Princu7 @geekyd I would suggest to resume this and make changes such that it works before the viewport.
@mariobehling The concept of lazy loading does not depend on internet, hence there is no problem for this in offline mode.

@aayusharora

This comment has been minimized.

Show comment
Hide comment
@aayusharora

aayusharora Mar 14, 2017

Member

@geekyd please concatenate files using IIFE

Member

aayusharora commented Mar 14, 2017

@geekyd please concatenate files using IIFE

@geekyd

This comment has been minimized.

Show comment
Hide comment
@geekyd

geekyd Mar 15, 2017

Member

@aayusharora I used IIFE to concat all the files but as i mentioned the js which are not used in the html gives error in the script, which is not desired.

Member

geekyd commented Mar 15, 2017

@aayusharora I used IIFE to concat all the files but as i mentioned the js which are not used in the html gives error in the script, which is not desired.

@aayusharora

This comment has been minimized.

Show comment
Hide comment
@aayusharora

aayusharora Mar 15, 2017

Member

Can we insert it pagewise than ?

Member

aayusharora commented Mar 15, 2017

Can we insert it pagewise than ?

@geekyd

This comment has been minimized.

Show comment
Hide comment
@geekyd

geekyd Mar 15, 2017

Member

Yes, we can make individual js for each page, that wont give any error and should work :)

Member

geekyd commented Mar 15, 2017

Yes, we can make individual js for each page, that wont give any error and should work :)

@aayusharora

This comment has been minimized.

Show comment
Hide comment
@aayusharora

aayusharora Mar 15, 2017

Member

@geekyd

  • Can you please change the threshold and generate a new PR, we don't want the user know, that something is happening.
  • Can you check the performance improvement after minification by gulp.
Member

aayusharora commented Mar 15, 2017

@geekyd

  • Can you please change the threshold and generate a new PR, we don't want the user know, that something is happening.
  • Can you check the performance improvement after minification by gulp.
@geekyd

This comment has been minimized.

Show comment
Hide comment
@geekyd

geekyd Mar 16, 2017

Member

@aayusharora I'm on it.

Member

geekyd commented Mar 16, 2017

@aayusharora I'm on it.

@aayusharora

This comment has been minimized.

Show comment
Hide comment
@aayusharora

aayusharora Mar 16, 2017

Member
Member

aayusharora commented Mar 16, 2017

aayusharora added a commit that referenced this issue Mar 18, 2017

Merge pull request #1183 from geekyd/gulpify
Adds gulp integration for issue #1105
@geekyd

This comment has been minimized.

Show comment
Hide comment
@geekyd

geekyd Mar 18, 2017

Member

@aayusharora Please re-open this issue. Thanks

Member

geekyd commented Mar 18, 2017

@aayusharora Please re-open this issue. Thanks

@mariobehling

This comment has been minimized.

Show comment
Hide comment
@mariobehling

mariobehling Feb 21, 2018

Member

Many improvements have been made. Follow up issues should go more into specifics.

Member

mariobehling commented Feb 21, 2018

Many improvements have been made. Follow up issues should go more into specifics.

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