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

Make drop zones larger when dragging in the same section #2206

Merged
merged 13 commits into from
Aug 18, 2022

Conversation

sroy3
Copy link
Contributor

@sroy3 sroy3 commented Aug 17, 2022

Closes #2115

Screen.Recording.2022-08-17.at.3.34.28.PM.mov
Screen.Recording.2022-08-17.at.3.54.44.PM.mov

@sroy3 sroy3 added the bug Something isn't working label Aug 17, 2022
@sroy3 sroy3 self-assigned this Aug 17, 2022
@sroy3 sroy3 marked this pull request as ready for review August 17, 2022 20:20
@@ -72,33 +72,44 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
// eslint-disable-next-line sonarjs/cognitive-complexity
}) => {
const [draggedOverId, setDraggedOverId] = useState('')
const [hoveringSomething, setHoveringSomething] = useState(false)
Copy link
Member

Choose a reason for hiding this comment

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

[Q] what is the difference between hoveringSomething and isHovering, why do we need both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are working together. And it's because of:

const handleDragLeave = () => {
    isHovering.current = false
    hoveringTimeout.current = window.setTimeout(() => {
      if (!isHovering.current) {
        setHoveringSomething(false)
      }
    }, 500)
  }

onDragLeave is a little too reactive and changing the state every time is a lot. Not only that but there is also a race condition with onDragEnter of other elements.

@sroy3 sroy3 enabled auto-merge (squash) August 18, 2022 13:43
@codeclimate
Copy link

codeclimate bot commented Aug 18, 2022

Code Climate has analyzed commit 98b5d8b and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 89.5% (85% is the threshold).

This pull request will bring the total coverage in the repository to 96.6% (-0.1% change).

View more on Code Climate.

@sroy3 sroy3 merged commit 92f86d1 into main Aug 18, 2022
@sroy3 sroy3 deleted the large-drop-same-section branch August 18, 2022 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problems with drag and drop targets in plots
2 participants