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

Fix a typo made comparing previous prop #7628

Merged
merged 1 commit into from
Dec 3, 2019
Merged

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Dec 3, 2019

The prop prevIsAccountMenuOpen was referenced in prevProps, despite it not existing. It seems clear from the context that the intention was to check the isAccountMenuOpen prop from prevProps, and name the local variable prevIsAccountMenuOpen.

Thanks to @desfero for catching this!

The prop `prevIsAccountMenuOpen` was referenced in `prevProps`, despite
it not existing. It seems clear from the context that the intention
was to check the `isAccountMenuOpen` prop from `prevProps`, and name
the local variable `prevIsAccountMenuOpen`.
@whymarrh
Copy link
Contributor

whymarrh commented Dec 3, 2019

Interesting—how did this work before? Did this manifest itself in a bug?

@Gudahtt
Copy link
Member Author

Gudahtt commented Dec 3, 2019

I believe the only impact of this was that the setAtAccountListBottom function was called every single render, rather than just when the account menu opened or closed. This function grabs the absolute position of the bottom of the account menu from the DOM determines whether the component is scrolled all the way down or not, then stores that boolean in component state. Calling it more often shouldn't have caused any bugs - it's just bad for performance.

@metamaskbot
Copy link
Collaborator

Builds ready [9628e3e]

@Gudahtt Gudahtt merged commit b4fd7ae into develop Dec 3, 2019
@whymarrh whymarrh deleted the fix-previous-prop-typo branch December 3, 2019 19:33
Gudahtt added a commit that referenced this pull request Dec 3, 2019
* origin/develop:
  Fix SimpleDropdown event bubbling (#7627)
  Remove unneeded store connections (#7625)
  Replace wild random number key with index (#7629)
  Use ES6 exports for selectors (#7626)
  Fix a typo made comparing previous prop (#7628)
  Connect distinct accounts per site (#7004)
  eslint: Enable more react/jsx-* rules (#7592)
  Enable react/no-unused-state rule for ESLint (#7609)
  Enable no-var rule for ESLint (#7590)
  Process URL fragment for ens-ipfs redirects (#7604)
  add locale fix script (#7580)
  Prevent redux state mutation (#7598)
Gudahtt added a commit that referenced this pull request Dec 5, 2019
The prop `prevIsAccountMenuOpen` was referenced in `prevProps`, despite
it not existing. It seems clear from the context that the intention
was to check the `isAccountMenuOpen` prop from `prevProps`, and name
the local variable `prevIsAccountMenuOpen`.
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.

3 participants