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 this.getInitialState is not a function err in DropZone component #7365

Merged
merged 2 commits into from
Aug 9, 2016

Conversation

lamosty
Copy link
Contributor

@lamosty lamosty commented Aug 9, 2016

Fixes #7353. During refactoring the DropZone component, getInitialState method was removed. However, its call in resetDragState method was not removed which causes 2169 errors in 24 hour period (thanks @artpi for your work on JS error logging!).

In this PR, I propose a simple fix in which we reset state in resetDragState more verbosely by enumerating specific props and their initial values instead of calling some method. Props @aduth for this solution and help.

I also reuse the resetDragState method in onDrop event handler as the resetting functionality was unnecessarily duplicated there.

Testing Instructions

To be honest, I couldn't reproduce the issue the "normal" way. I couldn't get the mouseup event triggered by using Calypso (maybe you could, here are some steps).

Fortunately, we can create and fire events manually through Dev Console. So, firstly, let's reproduce the bug and then see if it's fixed:

  1. Checkout master branch and open client/components/drop-zone/index.jsx in your favorite editor;
  2. Comment/delete these lines (they prevent us from seeing the error with the manual event firing);
  3. Load Calypso Media Library or just create a new post;
  4. Open Dev Tools and copy paste the following code in the Console tab:
var event = document.createEvent("HTMLEvents");

event.initEvent("mouseup", true, true);

window.dispatchEvent(event);
  1. Notice the this.getInitialState is not a function error;
  2. Checkout this branch and do the same;
  3. Is there still an error?

cc @aduth @ockham @gwwar @timmyc @artpi

Test live: https://calypso.live/?branch=fix/drop-zone-reset-drag-state

@lamosty lamosty added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Post/Page Editor The editor for editing posts and pages. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Feature] Media The media screen in Calypso, general media management, or integration with third party media. labels Aug 9, 2016
@lamosty lamosty self-assigned this Aug 9, 2016
@gwwar
Copy link
Contributor

gwwar commented Aug 9, 2016

👍 Code makes sense and tests well. :shipit: Nice find from @artpi's js error logging

@gwwar gwwar added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Aug 9, 2016
@lamosty
Copy link
Contributor Author

lamosty commented Aug 9, 2016

Thanks for the review!

@lamosty lamosty merged commit 1735e99 into master Aug 9, 2016
@lamosty lamosty deleted the fix/drop-zone-reset-drag-state branch August 9, 2016 19:42
@artpi
Copy link
Contributor

artpi commented Aug 10, 2016

Splendid! I am excited to get JS-Error logging back and running to see if the error is gone :)

@lamosty
Copy link
Contributor Author

lamosty commented Aug 10, 2016

I am excited to get JS-Error logging back and running to see if the error is gone

Me too! The thing is, there can be some other very rare errors we overlooked in this part of codebase. The JS error logging utility is really helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Media The media screen in Calypso, general media management, or integration with third party media. [Feature] Post/Page Editor The editor for editing posts and pages. [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

this.getInitialState is not a function
3 participants