Skip to content
This repository has been archived by the owner. It is now read-only.

remove unnecessary `frames` property from `closeWindow` #8256

Merged
merged 1 commit into from Apr 13, 2017
Merged

Conversation

@bridiver
Copy link
Collaborator

bridiver commented Apr 12, 2017

cc @bbondy

@bridiver bridiver added the perf label Apr 12, 2017
@bridiver bridiver requested review from bsclifton and NejcZdovc Apr 12, 2017
@bridiver bridiver added this to the 0.14.3 milestone Apr 12, 2017
@@ -1002,11 +997,9 @@ class Main extends ImmutableComponent {
sortedFrames.map((frame) =>
<Frame
ref={(node) => { this.frames[frame.get('key')] = node }}
tabData={this.props.appState.get('tabs').find((tab) => tab.get('tabId') === frame.get('tabId'))}

This comment has been minimized.

Copy link
@bridiver

bridiver Apr 12, 2017

Author Collaborator

this was just a random prop that wasn't being used anywhere

This comment has been minimized.

Copy link
@bbondy

bbondy Apr 13, 2017

Member

strange, I bet it was at some point and not anymore.

This comment has been minimized.

Copy link
@bsclifton

bsclifton Apr 13, 2017

Member

@bridiver I don't remember the original use-case, but I did use this when implementing the per-tab alerts (but then removed in favor of a distinct messageBoxDetail property- part of tab object)

@bridiver bridiver requested a review from bbondy Apr 12, 2017
@bbondy
Copy link
Member

bbondy commented Apr 13, 2017

++ pls merge as long as tests pass

@NejcZdovc
Copy link
Member

NejcZdovc commented Apr 13, 2017

@bbondy test are failing, but they are failing for master as well, so it's not related to this change

@NejcZdovc NejcZdovc merged commit b887fbe into master Apr 13, 2017
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@cezaraugusto cezaraugusto deleted the frame-cleanup branch Jun 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.