bbc / lrud

Left, Right, Up, Down. A spatial navigation library for devices with input via directional controls.
Apache License 2.0
98 stars 21 forks source link

Empty nodes cause problems when trying to focus/navigate #26

Closed NickGoward closed 5 years ago

NickGoward commented 5 years ago

Describe the bug When a node contains no focusable items, LRUD throws an error when trying to find an appropriate focusable item.

Case 1 - initial focus

To Reproduce

const nav = new Lrud()

nav.registerNode('root', {
  orientation: 'vertical'
})

nav.registerNode('node1', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.registerNode('node2', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.register('item2', {
  parent: 'node2',
  selectAction: {}
})

nav.assignFocus('root')

Expected behavior Should focus on item2 but instead throws error: Uncaught TypeError: Cannot read property 'id' of undefined

Case 2 - moving to empty node

To Reproduce

const nav = new Lrud()

nav.registerNode('root', {
  orientation: 'vertical'
})

nav.registerNode('node1', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.registerNode('node2', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.register('item1', {
  parent: 'node1',
  selectAction: {}
})

nav.assignFocus('root')

nav.handleKeyEvent({ direction: 'down' })

Expected behavior Should remain focused on item1 but instead throws error: Uncaught TypeError: Cannot read property 'id' of undefined

Case 3 - moving past empty node to item beyond it

To Reproduce

const nav = new Lrud()

nav.registerNode('root', {
  orientation: 'vertical'
})

nav.registerNode('node1', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.registerNode('node2', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.registerNode('node3', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.register('item1', {
  parent: 'node1',
  selectAction: {}
})

nav.register('item3', {
  parent: 'node3',
  selectAction: {}
})

nav.assignFocus('root')

nav.handleKeyEvent({ direction: 'down' })

Expected behavior Should focus on item3 but instead throws error: Uncaught TypeError: Cannot read property 'id' of undefined

thomascgray commented 5 years ago

Thanks for raising to our attention! And cheers for raising the issue with reproducible test scenarios 👍 Unfortunately, we don't have capacity to tackle this at the minute. Feel free to raise a PR though? if you wanna do a PR, one of us (LH 🐴 ) can definitely spare some time to go over stuff or whatever