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

Make all components OnPush #5035

Closed
29 of 30 tasks
jelbourn opened this issue Jun 8, 2017 · 9 comments
Closed
29 of 30 tasks

Make all components OnPush #5035

jelbourn opened this issue Jun 8, 2017 · 9 comments
Assignees
Labels
P2 The issue is important to a large percentage of users, with a workaround

Comments

@jelbourn
Copy link
Member

jelbourn commented Jun 8, 2017

  • MdAutocomplete
  • MdAnchor
  • MdButtonToggle
  • MdOptgroup
  • MdOption
  • MdPseudoCheckbox
  • MdDatepicker
  • MdDialogContainer
  • MdExpansionPanelHeader
  • MdExpansionPanel
  • MdGridList
  • MdGridTile
  • MdGridTileText
  • MdInputContainer
  • MdList
  • MdListItem
  • MdMenu
  • MdMenuItem
  • MdSpinner
  • MdSelect
  • MdSlider
  • SimpleSnackBar
  • MdSnackBarContainer
  • MdTabBody
  • MdTabGroup
  • MdTabHeader
  • MdTab
  • MdTabNavBar
  • TooltipComponent
  • MdSidenavContainer

We should make these change incrementally, tackling one high-level component at a time (e.g., "tabs"). This will make it easier to revert in case something goes wrong after the change (and also help prevent a rebasing nightmare).

Couple of things worth noting:

  • Most components should be as simple as adding OnPush to the config. This is the case if the component just has simple inputs and outputs.
  • Anything that implements ControlValueAccessor needs markForCheck in the forms hooks
@jelbourn jelbourn added the P2 The issue is important to a large percentage of users, with a workaround label Jun 8, 2017
@alexw10
Copy link

alexw10 commented Jun 9, 2017

@jelbourn just curious as to why you guys are planning to do this?

@alexciesielski
Copy link

@alexw10 OnPush is a performance setting, so the component gets only rerendered by Angular when its inputs and/or outputs change

@jelbourn
Copy link
Member Author

jelbourn commented Jun 9, 2017

Yeah, it helps limit the cases where change detection runs on a component.

@fxck
Copy link
Contributor

fxck commented Jun 10, 2017

#3005
#3106

related onpush issues

@z639
Copy link

z639 commented Jun 19, 2017

and #5118

@e-cloud
Copy link
Contributor

e-cloud commented Jun 27, 2017

@jelbourn since there are so many components needed to be updated, your team should make the releases of Material2 more frequently, should'n you?

crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 29, 2017
Switches the dialog container to `OnPush` change detection.

Relates to angular#5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 29, 2017
* Switches the progress spinner component to `OnPush` change detection.
* Removes a workaround for older Angular 2.x versions where inherited lifecycle hooks weren't being called.
* Fixes some wrong terminology in the spinner demo.

Relates to angular#5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 5, 2017
* Switches the snack bar component to `OnPush` change detection.
* Removes the `detectChanges` workaround for the choppy animations, because it no longer works with `OnPush`. Properly fixes the initial issue by providing a `void` animation state.

Relates to angular#5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 5, 2017
Switches the grid list component to OnPush change detection.

Relates to angular#5035.
@crisbeto crisbeto self-assigned this Jul 5, 2017
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 5, 2017
Switches the `md-menu` and `md-menu-item` components to `OnPush` change detection.

Relates to angular#5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 5, 2017
Switches the `md-list` and `md-list-item` to `OnPush` change detection.

Relates to angular#5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 6, 2017
* Switches `md-autocomplete` to `OnPush` change detection.
* Fixes a newly-introduced test failure.

Relates to angular#5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 6, 2017
* Switches the expansion panel to `OnPush` change detection.
* Fixes the open animation not triggering on the body element.

Relates to angular#5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 6, 2017
* Switches `md-slider` to `OnPush` change detection.
* Fixes a few properties not reacting to external changes.

Relates to angular#5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 6, 2017
* Switches `md-slider` to `OnPush` change detection.
* Fixes a few properties not reacting to external changes.

Relates to angular#5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 6, 2017
Switches `md-tooltip` to `OnPush` change detection.

Relates to angular#5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 13, 2017
Switches the dialog container to `OnPush` change detection.

Relates to angular#5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 13, 2017
Switches the dialog container to `OnPush` change detection.

Relates to angular#5035.
jelbourn pushed a commit that referenced this issue Jul 13, 2017
Switches the `MdInputContainer` to `OnPush` change detection and sorts out the various change detection-related issues.

Relates to #5035.
jelbourn pushed a commit that referenced this issue Jul 13, 2017
Switches `md-select` to use `OnPush` change detection and adds a few necessary `markForCheck` calls.

Relates to #5035.
jelbourn pushed a commit that referenced this issue Jul 13, 2017
* Switches the button toggle component to OnPush change detection.
* Fixes an issue where some toggles wouldn't be unselected when moving between toggles.

* Relates to #5035.
jelbourn pushed a commit that referenced this issue Jul 13, 2017
Switches the dialog container to `OnPush` change detection.

Relates to #5035.
jelbourn pushed a commit that referenced this issue Jul 14, 2017
* Switches the button toggle component to OnPush change detection.
* Fixes an issue where some toggles wouldn't be unselected when moving between toggles.

* Relates to #5035.
jelbourn pushed a commit that referenced this issue Jul 14, 2017
Switches `md-select` to use `OnPush` change detection and adds a few necessary `markForCheck` calls.

Relates to #5035.
jelbourn pushed a commit that referenced this issue Jul 14, 2017
Switches the `MdSidenavContainer` to `OnPush` change detection and fixes some changes not being picked up when its sidenav is opened.

Relates to #5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 15, 2017
Switches the `MdInputContainer` to `OnPush` change detection and sorts out the various change detection-related issues.

Relates to angular#5035.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 20, 2017
Switches the `MdInputContainer` to `OnPush` change detection and sorts out the various change detection-related issues.

Relates to angular#5035.
kara pushed a commit that referenced this issue Jul 20, 2017
Switches the `MdInputContainer` to `OnPush` change detection and sorts out the various change detection-related issues.

Relates to #5035.
andrewseguin pushed a commit that referenced this issue Jul 24, 2017
Switches the `MdSidenavContainer` to `OnPush` change detection and fixes some changes not being picked up when its sidenav is opened.

Relates to #5035.
andrewseguin pushed a commit that referenced this issue Jul 24, 2017
Switches the `MdInputContainer` to `OnPush` change detection and sorts out the various change detection-related issues.

Relates to #5035.
andrewseguin pushed a commit that referenced this issue Jul 24, 2017
Switches the dialog container to `OnPush` change detection.

Relates to #5035.
andrewseguin pushed a commit that referenced this issue Jul 25, 2017
Switches the `MdSidenavContainer` to `OnPush` change detection and fixes some changes not being picked up when its sidenav is opened.

Relates to #5035.
andrewseguin pushed a commit that referenced this issue Jul 25, 2017
Switches the `MdInputContainer` to `OnPush` change detection and sorts out the various change detection-related issues.

Relates to #5035.
andrewseguin pushed a commit that referenced this issue Jul 25, 2017
Switches the dialog container to `OnPush` change detection.

Relates to #5035.
andrewseguin pushed a commit that referenced this issue Jul 25, 2017
Switches the `MdSidenavContainer` to `OnPush` change detection and fixes some changes not being picked up when its sidenav is opened.

Relates to #5035.
andrewseguin pushed a commit that referenced this issue Jul 25, 2017
* chore(input): switch to OnPush change detection

Switches the `MdInputContainer` to `OnPush` change detection and sorts out the various change detection-related issues.

Relates to #5035.

* chore: linter errors
andrewseguin pushed a commit that referenced this issue Jul 27, 2017
Switches the dialog container to `OnPush` change detection.

Relates to #5035.
andrewseguin pushed a commit that referenced this issue Jul 27, 2017
Switches the dialog container to `OnPush` change detection.

Relates to #5035.
andrewseguin pushed a commit that referenced this issue Jul 27, 2017
Switches the dialog container to `OnPush` change detection.

Relates to #5035.
andrewseguin pushed a commit that referenced this issue Jul 28, 2017
Switches the dialog container to `OnPush` change detection.

Relates to #5035.
@crisbeto
Copy link
Member

Closing since all components have been switched over now.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Development

No branches or pull requests

7 participants