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.

Sign up for GitHub

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: css grid drag and drop jank #174

Merged
merged 2 commits into from
Feb 5, 2021
Merged

fix: css grid drag and drop jank #174

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9be963b
fix: css grid drag and drop jank
timc1 Feb 3, 2021
f8f751e
Merge branch 'next' into fix/css-grid-dnd
prevwong Feb 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix: css grid drag and drop jank
  • Loading branch information
@timc1
timc1 committed Feb 3, 2021
commit 9be963b6b56296841e7a7652a02f6246411b7830
3 changes: 3 additions & 0 deletions packages/utils/src/getDOMInfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ export const styleInFlow = (el: HTMLElement, parent: HTMLElement) => {

if (style.overflow && style.overflow !== 'visible') return;
if (parentStyle.float !== 'none') return;
if (parent && parentStyle.display === 'grid') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trying to understand the mechanism under the hood for this logic. Why does flex specifically check for flex-direction: column? 🤔

Copy link
Owner

Choose a reason for hiding this comment

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

One of the oldest part of Craft 😄 So this part is related to the placement logic and it's mostly borrowed from GrapesJS.

Let's say we have 2 elements - A for the element we're dragging and B for the element we're hovering over.

If element B is in "flow" - then we would only consider B's y-axis threshold relative to the current cursor position to determine if A should be dropped before or after B. This is because some elements are arranged vertically, and so naturally when you're gonna drop something at these vertically arranged elements, you would move the cursor from top-to-bottom rather than from left-to-right.

For example, if a child element's parent is of flex-direction: column, it would mean that it takes up the entire width of its parent, and should be arranged vertically.

Related to: https://github.com/artf/grapesjs//issues/1626

Copy link
Contributor Author

@timc1 timc1 Feb 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, so this is a little difficult to calculate for grid as checking flow wouldn't be as straight forward as flex's flex-direction? We'd likely need something like this here or grid layouts have a terrible drop cursor experience 😄

Copy link
Owner

Choose a reason for hiding this comment

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

Yea, we would probably require some additional heuristics

Copy link
Owner

Choose a reason for hiding this comment

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

@timc1 Shall we merge this one in as a temporary solution for grid layouts? Then, we can revisit this issue with a more complete solution later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prevwong let's do that 👍

return;
}
if (
parent &&
parentStyle.display === 'flex' &&
Expand Down