Adding back code-highlight to blog post, tweaked performance issue with scrolling.

ustwo / ustwo.com-frontend

The New & Improved ustwo Website

http://www.ustwo.com

Other

1.82k stars 269 forks source link

Adding back code-highlight to blog post, tweaked performance issue with scrolling. #569

Open juancabrera opened 7 years ago

juancabrera commented 7 years ago

This PR covers:

Adding back code highlighting to blog posts, fixed issue with multiple nesting and multiple re-rendering.
Added throttle of 240ms to main onScroll event listener to help with performance.

Notes:

Code highlighting is working but it's trying to detect what language is for each block, as right now there is no way to pass the language. This eventually will need to get implemented for optimal results.

phil-linnell commented 7 years ago

@juancabrera 1) Great, nice work here. Do we need nextProps, nextState arguments as they are not used? 2) Much improved performance on pages that don't utilise the scroll listener which is nice. However, I'm not enjoying the clunky feeling on areas that DO rely on the scroll listener, e.g. scroll back to the top on home or work page. As a result I feel like we should approach this slightly differently. I need to get some time to look into how we can improve when and where we use the scroll listener, but please jump in with any thoughts.

Finally, the language support: I guess, @andydutch, this could be a feature to implement - some way to choose the language for each block in WP. At the moment the automatic detection is useless and I don't think we can justify the introduction of syntax highlighting unless we solve it. What do you think?

juancabrera commented 7 years ago

@phil-linnell agree with you. Do you think that lower down the throttle to maybe 100ms will help ?.

@andydutch let us know what you think! and how we can help to make this happen.

phil-linnell commented 7 years ago

@juancabrera Changing the throttle to 100ms didn't help. Based on my thoughts about performance elsewhere in the app, I thought maybe just to return to no throttling, and instead remove the listener on post and caseStudy pages. What do you think?

juancabrera commented 7 years ago

@phil-linnell gotcha.

about the listener, when I chatted with casey and fredrik one of the most useful things of the listener was to know if people finished (or how far read) the blog post. So based on that it should be on the post page. In fact, I tried removing it and works like a sharm.

they were open to change or other options, so let me check with them again. Other way to get a sense of how much the people reads the blogpost is by how much time they spent on the post.

will get back to you asap.