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

Editor: Fix resetting the drop-zone states after dropping a file #2604

Merged
merged 1 commit into from Sep 4, 2017

Conversation

youknowriad
Copy link
Contributor

closes #2597

The dropzone classes were not being cleaned on drop which causes the dropzone to appear over the controls.

Testings instructions

@youknowriad youknowriad added the [Type] Bug An existing feature does not function as intended label Aug 30, 2017
@youknowriad youknowriad self-assigned this Aug 30, 2017
@youknowriad youknowriad requested review from mapk and aduth August 30, 2017 08:39
@@ -68,7 +68,7 @@ class DropZoneProvider extends Component {
} );

this.dropzones.forEach( ( { updateState } ) => {
updateState( { isDraggingOverDocument, isDraggingOverElement: false, position: null } );
updateState( { isDraggingOverDocument: false, isDraggingOverElement: false, position: null } );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should consider splitting long line across multiple.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we definitely need prettier :trollface: one hour dev time gained per day :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

object-curly-newline has options for configuring number of properties before newlines are required, and includes a fixer!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you use the auto-fixer on IDE save? I never saw ESlint used for this, but yeah could be a good alternative for now 👍.

I'm using prettier "auto-save format" in other projects and it's really a huge gain.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't personally, but a cursory search reveals this one for Sublime Text:

https://github.com/TheSavior/ESLint-Formatter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a similar config in VS Core, I'll be trying it for some days. I'm not adding the rule right now, but we should consider it in a separate PR.

@codecov
Copy link

codecov bot commented Sep 4, 2017

Codecov Report

Merging #2604 into master will increase coverage by 0.32%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2604      +/-   ##
==========================================
+ Coverage   31.38%   31.71%   +0.32%     
==========================================
  Files         177      177              
  Lines        5413     5811     +398     
  Branches      949     1065     +116     
==========================================
+ Hits         1699     1843     +144     
- Misses       3139     3309     +170     
- Partials      575      659      +84
Impacted Files Coverage Δ
components/drop-zone/provider.js 1.33% <0%> (ø) ⬆️
blocks/library/latest-posts/index.js 7.4% <0%> (-2.6%) ⬇️
blocks/editable/index.js 9.85% <0%> (-0.68%) ⬇️
...ebar/post-taxonomies/hierarchical-term-selector.js 0% <0%> (ø) ⬆️
blocks/api/registration.js 100% <0%> (ø) ⬆️
editor/modes/visual-editor/block.js 0% <0%> (ø) ⬆️
editor/inserter/menu.js 48.46% <0%> (+0.05%) ⬆️
components/dropdown-menu/index.js 95.8% <0%> (+1.01%) ⬆️
blocks/library/button/index.js 28.57% <0%> (+1.9%) ⬆️
components/button/index.js 92.85% <0%> (+1.94%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 285f5ea...a02ef13. Read the comment docs.

@youknowriad youknowriad merged commit 51c2765 into master Sep 4, 2017
@youknowriad youknowriad deleted the fix/reset-drop-zones branch September 4, 2017 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Block tools not responding
2 participants