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.

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

Bolt Core Renderer + Rendering Animation Performance Comparison #964

Merged
merged 9 commits into from Nov 26, 2018

Conversation

sghoweri
Copy link
Contributor

@sghoweri sghoweri commented Nov 19, 2018

Jira

N/A

Summary

Adds 3 new demos to Bolt to highlight major rendering performance differences + a potential new technique to help address issues with scale -- particularly necessary with upcoming animation work.

Details

This PR also updates a few things in the build tools related to getting these demos up and running:

  1. Adds the ability to compile Typescript .ts, and .tsx files in Bolt's local Webpack Build
  2. Updates the @bolt/core click event handler util to allow toggling JavaScript methods that are on the window (vs only allowing methods that are on an element itself)
  3. Updates the @bolt/core base component to no longer immediately trigger a component without any props to render (via the updated() method call) -- fixes a minor issue discovered with the new Context API additions in Bolt Release v2.2.0 #961 that can cause components using the new withContext functionality to have undefined values initially until the component finishes rendering.

How to test

  • Confirm the build compiles as expected
  • Confirm existing click event handlers (ex. buttons hooked up to start playing the video player on click) continue to work as expected
  • Confirm the new rendering performance demos are working in modern browsers (Chrome, Safari and Firefox -- IE 11 support isn't yet possible due to the way slots are used in the demo components)

CC @mikemai2awesome @danielamorse @adam2661 -- particularly regarding the 3rd item above since I'm 99% sure Adam and I ran into this snag last week with the new List component work.

@bolt-bot
Copy link
Collaborator

⚡ PR built on Travis and deployed a now preview here:

@bolt-bot
Copy link
Collaborator

⚡ PR built on Travis and deployed a now preview here:

@danielamorse
Copy link
Collaborator

@sghoweri, please resolve conflicts.

@sghoweri
Copy link
Contributor Author

@danielamorse all set resolving merge conflicts!

@bolt-bot
Copy link
Collaborator

⚡ PR built on Travis and deployed a now preview here:

Copy link
Collaborator

@danielamorse danielamorse left a comment

Choose a reason for hiding this comment

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

Building finished with no issues.

Video play buttons work as expected on the following pages in Chrome, FF, and Safari.

Rendering demos worked as expected on the following pages (with and without throttling) in Chrome, FF, and Safari.

I noticed in Safari only this video did not auto-play:
http://localhost:3000/pattern-lab/?p=components-video-autoplay

While this one did:
http://localhost:3000/pattern-lab/?p=components-video-autoplay-and-loop-hide-controls

Same thing happens on Master, so it's not a new bug. Is this a known issue?

@sghoweri
Copy link
Contributor Author

@danielamorse yeah - autoplay support is a mixed bag (especially on mobile).

As long as the button component can trigger the video to start playing when clicked / tapped (which you confirmed), I’m good with moving forward with this. Thanks!

@sghoweri sghoweri merged commit 6b08652 into master Nov 26, 2018
@sghoweri sghoweri deleted the feature/animation-rendering-performance branch November 26, 2018 20:41
@sghoweri sghoweri mentioned this pull request Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants