Skip to content

Move items with keyboard controls #3092

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

CharlieWinkwaves
Copy link

Description

We are trying to get our application to comply to the a11y norm. One of the issues we encountered was that gridstack is not accessible with the keyboard.
A user cannot select and move items around with only the use of a keyboard.
What we have done is reuse the moving of items for the mouse for the keyboard.

This works, except when using items with different heights.
When moving an item up or down and the item above or below has a greater height, then the item does not move.
As we use the items height to move the item, I know that the position is only changed when it moves further than half way the other item. Is there a way we can know how far to move the item to make sure it changes position?

Checklist

  • Created tests which fail without the change (if possible)
  • All tests passing (yarn test)
  • Extended the README / documentation, if necessary

When using the arrow keys the item moves one colum left or right. Or its
own height up or down.
The item is moved with the keyboard and the item stays in view of the
user.
@adumesny
Copy link
Member

this is great and will need to take a look, but when using arrow keys we need to move entire elements to either side to replace with, because unlike the mouse we cannot incrementally move and wait for half way point to initiate a move. using arrow keys it shoudl always move in that direction so code needs to check what is say the element above and move itself to the beginning of that element top postion and let the code figure out what it means (swap, push down, etc...). You shoudl be able to do that using update(el) and placing where the other target is instead.

@CharlieWinkwaves
Copy link
Author

CharlieWinkwaves commented Jul 14, 2025

I tried it with update, and things move around only they move behind the other items they do not update the position of the other elements. I removed all the calls to trigger the mouseEvents. This is what you meant right ? I can probably figure out which items overlap but how to determine the position they need to move to ? we use items with different heights and widths. So it not always as easy to say just swap them around.
`
protected _keyMove(e: KeyboardEvent) {
if (e.code === 'Space') {
e.preventDefault()

  this.keyboardSelected.classList.remove('grid-stack-item-selected')
  this.keyboardSelected.dispatchEvent(new MouseEvent('mouseup'))
  document.removeEventListener('keydown', this._keyMove)

  return
}

if (e.code === 'ArrowRight' ||
  e.code === 'ArrowLeft' ||
  e.code === 'ArrowUp' ||
  e.code === 'ArrowDown') {
  e.preventDefault()

  this._itemPosition(e.code)

  this.keyboardSelected = this._selectedItem(this.keyboardSelected)
  this.keyboardSelected.scrollIntoView({ block: "center" })
}

}

protected _itemPosition(code: string) {
const grid = this.el.gridstackNode?.grid
const node = this.el.gridstackNode
const maxColumn = node.grid.opts.column
const cellHeight = node.h

switch (code) {
case 'ArrowRight':
  if(typeof(maxColumn) == 'number' && node.x === (maxColumn - 1)) { break }

  node.x = node.x + 1
  break
case 'ArrowLeft':
  if (node.x === 0) { break }

  node.x = node.x - 1

  break
case 'ArrowUp':
  if (node.y === 0) { break }

  node.y = node.y - cellHeight
  break
case 'ArrowDown':
  node.y = node.y + cellHeight

  break
}

grid.update(this.keyboardSelected, node)

// grid.commit() / grid.save() both dont do anything
}
`

@CharlieWinkwaves
Copy link
Author

I figured somethings out without using update, maybe it will work with update as well. I will update this branch tomorrow

@CharlieWinkwaves
Copy link
Author

I fixed a lot of edge cases we had.
i also tried this with update but i still had the same issue as before the item moves but it moves behind the other elements and nothing updates

This takes the selected elements width and its position to check for the
first item above. When no direct item is found find the first item in
the row above and move the item above that.
First look for the next item in the row directly below the selected
item. When nothing is find look for the first item below the selected
item. And when that fails because of whitespacing, look for the first
item which is below and overlaps the selected item.

Also check if the selected item has sibling, because when the item below
spans more columns, it can mean there is no space for that item.
Instead, move the selected item below the next item.
@CharlieWinkwaves CharlieWinkwaves force-pushed the MoveItemsWithKeysboardControls branch from 60ac268 to 78dfd52 Compare July 16, 2025 15:12
Copy link
Member

@adumesny adumesny left a comment

Choose a reason for hiding this comment

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

unfortunately I have to completely re-write this the proper way from scratch, which could be faster than trying to fix your code.

}

protected _keyUp() {
document.removeEventListener('keyup', this._keyUp)
Copy link
Member

Choose a reason for hiding this comment

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

this is reeally hacky. youa re waiting for the hopefully space up even to track key down to now move...


const handle = e.target as HTMLElement
const item: HTMLElement = handle?.closest('.grid-stack-item')
this.keyboardSelected = item
Copy link
Member

Choose a reason for hiding this comment

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

you not preventing multiuple space event from doing same code and fakinga. mouseDown event which is hacky at best...

screenY: screenY }
}

_setCoordinates(element: HTMLElement, x: number, y:number) {
Copy link
Member

Choose a reason for hiding this comment

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

honestly all those _ methods (reserved to private) being public and no doc, and likely on teh wrong class or not eneded is not the lib style. I cannot use this code.

}

// Check if the selectedNode has any siblings to the left or right
_findSiblings(itemBelow: Element) {
Copy link
Member

Choose a reason for hiding this comment

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

there are util method to return hit target used for collision. not need for all this code....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants