atlassian / pragmatic-drag-and-drop

Fast drag and drop for any experience on any tech stack
https://atlassian.design/components/pragmatic-drag-and-drop
Other
9.65k stars 216 forks source link

Hitbox: Use logical `start` and `end` positioning values for `Edge` #31

Open hellovietduc opened 7 months ago

hellovietduc commented 7 months ago

Would it be a good idea to use logical positioning values for the hitbox detection? Would be nice to have this handled by Pragmatic drag and drop, otherwise we would have to flip the left and right value ourselves for RTL layout.

alexreardon commented 7 months ago

Interesting! I am not sure if logical properties would be cleaner for hitbox information 🤔 . It would be good to see some code to see how using logical properties rather than 'left' / 'right' would clean things up

hellovietduc commented 7 months ago

I use logical properties to position the drop indicator, in RTL layout this code would make it show up in the opposite side of the closest edge.

const EDGE_TO_POSITIONAL_PROPERTY_MAP: Record<Edge, keyof CSSProperties> = {
  top: 'top',
  bottom: 'bottom',
  left: 'inset-inline-start',
  right: 'inset-inline-end',
}

The fixed code would look something like this:

const EDGE_TO_POSITIONAL_PROPERTY_MAP: Record<Edge, keyof CSSProperties> = {
  top: 'top',
  bottom: 'bottom',
  ...(isRtl ? {
      left: 'inset-inline-end',
      right: 'inset-inline-start',
  } : {
      left: 'inset-inline-start',
      right: 'inset-inline-end',
  })
}

But it would be nice to not having to do this check everywhere. I have come up with this code that works pretty well:

type LogicalEdge = 'top' | 'end' | 'bottom' | 'start'

const extractLogicalClosestEdge = (target: DropTargetRecord): LogicalEdge | null => {
  const { element, data } = target
  const isRtl = element.closest('[dir="rtl"]') != null
  const closestEdge = extractClosestEdge(data)
  if (closestEdge == null) return null
  if (closestEdge === 'top' || closestEdge === 'bottom') return closestEdge
  if (closestEdge === 'left') return isRtl ? 'end' : 'start'
  return isRtl ? 'start' : 'end'
}

What do you think?