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

UIP-2637 Release over_react 1.16.1 (HOTFIX) #108

Conversation

greglittlefield-wf
Copy link
Contributor

@greglittlefield-wf greglittlefield-wf commented Aug 14, 2017

Fix case where setState and store trigger only result in one render

Ultimate problem:

Currently, when a component calls setState at the same time the store triggers, the rerender with the updated store does not take place.

How it was fixed:

Use componentWillReceiveProps to mark batch redraws instead of componentDidUpdate.

This will mark only components that are getting excessive rerenders from parent components, and not components that have unrelated setState calls.

Testing suggestions:

Potential areas of regression:

FluxUiComponent batch redrawing.


@Workiva/ui-platform-pp @Workiva/web-platform-pp @kylemcmorrow-wf

@aviary2-wf
Copy link

aviary2-wf commented Aug 14, 2017

Raven

Number of Findings: 0

@jacehensley-wf
Copy link
Contributor

+1

@greglittlefield-wf greglittlefield-wf changed the title Fix case where setState and store trigger only result in one render Release over_react 1.16.1 Aug 14, 2017
@greglittlefield-wf greglittlefield-wf changed the title Release over_react 1.16.1 Release over_react 1.16.1 (HOTFIX) Aug 14, 2017
@jacehensley-wf
Copy link
Contributor

+1

@greglittlefield-wf greglittlefield-wf changed the title Release over_react 1.16.1 (HOTFIX) UIP-2637 Release over_react 1.16.1 (HOTFIX) Aug 14, 2017
@codecov-io
Copy link

Codecov Report

Merging #108 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #108   +/-   ##
=======================================
  Coverage   94.37%   94.37%           
=======================================
  Files          31       31           
  Lines        1544     1544           
=======================================
  Hits         1457     1457           
  Misses         87       87

@leviwith-wf
Copy link
Contributor

QA +10

  • over_react tests pass
  • verified noted graph_app test fails without this change, passes with it

  • Dev +1’s on MPR
  • Dev +1’s present on related CRs
  • Version info updated
  • Rosie ran / comment displays expected info
  • Dependency Scan Clean

Ready for merge and tag.

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

Successfully merging this pull request may close these issues.

7 participants