Skip to content

Conversation

@jim-chou-shopify
Copy link
Contributor

@jim-chou-shopify jim-chou-shopify commented Apr 10, 2019

WHY are these changes introduced?

Improve performance of Resizer to avoid forced synchronous layout.

Screen Shot 2019-04-11 at 6 37 43 PM

WHAT is this pull request doing?

Schedule handleHeightCheck to run in next animation frame

As the JavaScript runs all the old layout values from the previous frame are known and available for you to query.

https://developers.google.com/web/fundamentals/performance/rendering/avoid-large-complex-layouts-and-layout-thrashing

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import * as React from 'react';
import {Page} from '@shopify/polaris';

interface State {}

export default class Playground extends React.Component<never, State> {
  render() {
    return (
      <Page title="Playground">
        {/* Add the code you want to test here */}
      </Page>
    );
  }
}

🎩 checklist

@darrenhebner

@BPScott BPScott temporarily deployed to polaris-react-pr-1301 April 10, 2019 16:43 Inactive
@jim-chou-shopify jim-chou-shopify force-pushed the measure-dom-style-in-frame branch from 113556e to e6c87a1 Compare April 10, 2019 16:46
@BPScott BPScott temporarily deployed to polaris-react-pr-1301 April 10, 2019 16:46 Inactive
@jim-chou-shopify jim-chou-shopify force-pushed the measure-dom-style-in-frame branch from e6c87a1 to 6f80e91 Compare April 10, 2019 16:53
@BPScott BPScott temporarily deployed to polaris-react-pr-1301 April 10, 2019 16:53 Inactive
@jim-chou-shopify jim-chou-shopify force-pushed the measure-dom-style-in-frame branch from 6f80e91 to 7c19a80 Compare April 11, 2019 14:15
@BPScott BPScott temporarily deployed to polaris-react-pr-1301 April 11, 2019 14:15 Inactive
@jim-chou-shopify jim-chou-shopify requested review from a team and darrenhebner and removed request for BPScott and ismail-syed April 11, 2019 22:40
Copy link
Member

@chloerice chloerice left a comment

Choose a reason for hiding this comment

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

LGTM 🚢 This is awesome, def gonna look into doing this in the data table 🤓

re: the codecov check, I think once you rebase it'll pass

@jim-chou-shopify jim-chou-shopify force-pushed the measure-dom-style-in-frame branch from 7c19a80 to 9c9151e Compare April 18, 2019 18:22
@BPScott BPScott temporarily deployed to polaris-react-pr-1301 April 18, 2019 18:22 Inactive
@jim-chou-shopify jim-chou-shopify force-pushed the measure-dom-style-in-frame branch from 9c9151e to 49c257d Compare April 18, 2019 19:44
@BPScott BPScott temporarily deployed to polaris-react-pr-1301 April 18, 2019 19:44 Inactive
@jim-chou-shopify jim-chou-shopify force-pushed the measure-dom-style-in-frame branch from 49c257d to c05187e Compare April 18, 2019 19:50
@BPScott BPScott temporarily deployed to polaris-react-pr-1301 April 18, 2019 19:50 Inactive
@jim-chou-shopify jim-chou-shopify force-pushed the measure-dom-style-in-frame branch from c05187e to 3c1025f Compare April 18, 2019 21:39
@BPScott BPScott temporarily deployed to polaris-react-pr-1301 April 18, 2019 21:39 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1301 April 18, 2019 22:04 Inactive
@jim-chou-shopify jim-chou-shopify force-pushed the measure-dom-style-in-frame branch from 8007a7c to f2e0f97 Compare April 19, 2019 02:35
@BPScott BPScott temporarily deployed to polaris-react-pr-1301 April 19, 2019 02:36 Inactive
@jim-chou-shopify jim-chou-shopify force-pushed the measure-dom-style-in-frame branch from f2e0f97 to d10416b Compare April 19, 2019 03:25
@BPScott BPScott temporarily deployed to polaris-react-pr-1301 April 19, 2019 03:26 Inactive
componentDidMount() {
this.handleHeightCheck();

if (process.env.NODE_ENV === 'development') {
Copy link
Contributor Author

@jim-chou-shopify jim-chou-shopify Apr 19, 2019

Choose a reason for hiding this comment

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

Verified in playground that it works in dev without timeout and tests passed.

@jim-chou-shopify jim-chou-shopify force-pushed the measure-dom-style-in-frame branch from d10416b to 167dfc4 Compare April 19, 2019 03:40
}

const contentHeight = this.contentNode.offsetHeight;
const minimumHeight = this.setMinimumLinesNode
Copy link
Contributor Author

@jim-chou-shopify jim-chou-shopify Apr 19, 2019

Choose a reason for hiding this comment

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

This condition is always true, because this.setMinimumLinesNode is a function and Boolean(Function) is truthy.

@jim-chou-shopify jim-chou-shopify merged commit 11fc996 into master Apr 22, 2019
@alex-page alex-page temporarily deployed to production April 22, 2019 19:26 Inactive
@kaelig kaelig deleted the measure-dom-style-in-frame branch April 26, 2019 20:55
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.

4 participants